Skip to content

Commit d7cadf9

Browse files
ksperling-applehasty
authored andcommitted
Dump details about leaked ExchangeContexts before aborting (project-chip#34617)
* Dump details about leaked ExchangeContexts before aborting This is implemented via a VerifyOrDieWithObject() variant of the existing VerifyOrDie() macro that calls a DumpToLog() method on the provided object if it exists (otherwise this is simply a no-op). If CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE is not enabled, VerifyOrDieWithObject() simply behaves like a plain VerifyOrDie(). DumpToLog() implementations can use ChipLogFormatRtti to log type information about an object (usually a delegate); if RTTI is disabled this simply outputs whether the object was null or not. * Address review comments * Make gcc happy and improve documentation * Remove unused include * Fix compile error without CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE * Avoid unused parameter warning
1 parent 78dab72 commit d7cadf9

File tree

9 files changed

+172
-8
lines changed

9 files changed

+172
-8
lines changed

src/lib/support/BUILD.gn

+5-1
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ source_set("logging_constants") {
6868

6969
source_set("attributes") {
7070
sources = [
71+
"Compiler.h",
7172
"DLLUtil.h",
7273
"EnforceFormat.h",
7374
]
@@ -132,7 +133,10 @@ source_set("text_only_logging") {
132133
}
133134

134135
source_set("verifymacros") {
135-
sources = [ "CodeUtils.h" ]
136+
sources = [
137+
"CodeUtils.h",
138+
"ObjectDump.h",
139+
]
136140

137141
public_deps = [
138142
":attributes",

src/lib/support/CodeUtils.h

+16
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <lib/core/CHIPConfig.h>
3030
#include <lib/core/CHIPError.h>
3131
#include <lib/core/ErrorStr.h>
32+
#include <lib/support/ObjectDump.h>
3233
#include <lib/support/VerificationMacrosNoLogging.h>
3334
#include <lib/support/logging/TextOnlyLogging.h>
3435

@@ -547,6 +548,21 @@ inline void chipDie(void)
547548
#define VerifyOrDie(aCondition) VerifyOrDieWithoutLogging(aCondition)
548549
#endif // CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE
549550

551+
/**
552+
* @def VerifyOrDieWithObject(aCondition, aObject)
553+
*
554+
* Like VerifyOrDie(), but calls DumpObjectToLog()
555+
* on the provided object on failure before aborting
556+
* if CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE is enabled.
557+
*/
558+
#if CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE
559+
#define VerifyOrDieWithObject(aCondition, aObject) \
560+
nlABORT_ACTION(aCondition, ::chip::DumpObjectToLog(aObject); \
561+
ChipLogError(Support, "VerifyOrDie failure at %s:%d: %s", __FILE__, __LINE__, #aCondition))
562+
#else // CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE
563+
#define VerifyOrDieWithObject(aCondition, aObject) VerifyOrDieWithoutLogging(aCondition)
564+
#endif // CHIP_CONFIG_VERBOSE_VERIFY_OR_DIE
565+
550566
/**
551567
* @def VerifyOrDieWithMsg(aCondition, aModule, aMessage, ...)
552568
*

src/lib/support/Compiler.h

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/*
2+
* Copyright (c) 2024 Project CHIP Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
// CHIP_HAVE_RTTI: Is C++ RTTI enabled?
18+
#if defined(__clang__)
19+
#define CHIP_HAVE_RTTI __has_feature(cxx_rtti)
20+
#elif defined(__GNUC__) && defined(__GXX_RTTI)
21+
#define CHIP_HAVE_RTTI 1
22+
#else
23+
#define CHIP_HAVE_RTTI 0
24+
#endif

src/lib/support/ObjectDump.h

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
*
3+
* Copyright (c) 2024 Project CHIP Authors
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
#pragma once
19+
20+
#include <type_traits>
21+
#include <utility>
22+
23+
namespace chip {
24+
25+
/**
26+
* A dumpable object that can log some useful state for debugging in fatal
27+
* error scenarios by exposing a `void DumpToLog() const` method. The method
28+
* should log key details about the state of object using ChipLogError().
29+
*/
30+
template <class, class = void>
31+
struct IsDumpable : std::false_type
32+
{
33+
};
34+
template <class T>
35+
struct IsDumpable<T, std::void_t<decltype(std::declval<T>().DumpToLog())>> : std::true_type
36+
{
37+
};
38+
39+
struct DumpableTypeExample
40+
{
41+
void DumpToLog() const {};
42+
};
43+
static_assert(IsDumpable<DumpableTypeExample>::value);
44+
45+
/**
46+
* Calls DumpToLog() on the object, if supported.
47+
*/
48+
template <class T>
49+
void DumpObjectToLog([[maybe_unused]] const T * object)
50+
{
51+
if constexpr (IsDumpable<T>::value)
52+
{
53+
object->DumpToLog();
54+
}
55+
}
56+
57+
} // namespace chip

src/lib/support/Pool.h

+27-2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
#include <lib/support/CHIPMem.h>
2626
#include <lib/support/CodeUtils.h>
27+
#include <lib/support/ObjectDump.h>
2728
#include <system/SystemConfig.h>
2829

2930
#include <lib/support/Iterators.h>
@@ -263,7 +264,7 @@ class BitMapObjectPool : public internal::StaticAllocatorBitmap
263264
{
264265
public:
265266
BitMapObjectPool() : StaticAllocatorBitmap(mData.mMemory, mUsage, N, sizeof(T)) {}
266-
~BitMapObjectPool() { VerifyOrDie(Allocated() == 0); }
267+
~BitMapObjectPool() { VerifyOrDieWithObject(Allocated() == 0, this); }
267268

268269
BitmapActiveObjectIterator<T> begin() { return BitmapActiveObjectIterator<T>(this, FirstActiveIndex()); }
269270
BitmapActiveObjectIterator<T> end() { return BitmapActiveObjectIterator<T>(this, N); }
@@ -323,6 +324,18 @@ class BitMapObjectPool : public internal::StaticAllocatorBitmap
323324
return ForEachActiveObjectInner(&proxy, &internal::LambdaProxy<T, Function>::ConstCall);
324325
}
325326

327+
void DumpToLog() const
328+
{
329+
ChipLogError(Support, "BitMapObjectPool: %lu allocated", static_cast<unsigned long>(Allocated()));
330+
if constexpr (IsDumpable<T>::value)
331+
{
332+
ForEachActiveObject([](const T * object) {
333+
object->DumpToLog();
334+
return Loop::Continue;
335+
});
336+
}
337+
}
338+
326339
private:
327340
static Loop ReleaseObject(void * context, void * object)
328341
{
@@ -389,7 +402,7 @@ class HeapObjectPool : public internal::Statistics, public HeapObjectPoolExitHan
389402
if (!sIgnoringLeaksOnExit)
390403
{
391404
// Verify that no live objects remain, to prevent potential use-after-free.
392-
VerifyOrDie(Allocated() == 0);
405+
VerifyOrDieWithObject(Allocated() == 0, this);
393406
}
394407
#endif // __SANITIZE_ADDRESS__
395408
}
@@ -570,6 +583,18 @@ class HeapObjectPool : public internal::Statistics, public HeapObjectPoolExitHan
570583
return mObjects.ForEachNode(&proxy, &internal::LambdaProxy<const T, Function>::ConstCall);
571584
}
572585

586+
void DumpToLog() const
587+
{
588+
ChipLogError(Support, "HeapObjectPool: %lu allocated", static_cast<unsigned long>(Allocated()));
589+
if constexpr (IsDumpable<T>::value)
590+
{
591+
ForEachActiveObject([](const T * object) {
592+
object->DumpToLog();
593+
return Loop::Continue;
594+
});
595+
}
596+
}
597+
573598
private:
574599
static Loop ReleaseObject(void * context, void * object)
575600
{

src/lib/support/logging/TextOnlyLogging.h

+20
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636

3737
#include <lib/core/CHIPConfig.h>
3838

39+
#include <lib/support/Compiler.h>
3940
#include <lib/support/DLLUtil.h>
4041
#include <lib/support/EnforceFormat.h>
4142
#include <lib/support/VerificationMacrosNoLogging.h>
@@ -44,6 +45,7 @@
4445
#include <inttypes.h>
4546
#include <stdarg.h>
4647
#include <stdint.h>
48+
#include <typeinfo>
4749

4850
#if CHIP_SYSTEM_CONFIG_PLATFORM_LOG && defined(CHIP_SYSTEM_CONFIG_PLATFORM_LOG_INCLUDE)
4951
#include CHIP_SYSTEM_CONFIG_PLATFORM_LOG_INCLUDE
@@ -292,6 +294,24 @@ using LogRedirectCallback_t = void (*)(const char * module, uint8_t category, co
292294
#define ChipLogValueExchangeIdFromReceivedHeader(payloadHeader) \
293295
ChipLogValueExchangeId((payloadHeader).GetExchangeID(), !(payloadHeader).IsInitiator())
294296

297+
/**
298+
* Logging helpers for logging the dynamic type of an object, if possible.
299+
*
300+
* Primarily useful when logging the type of delegates or similar objects when
301+
* performing logging for a fatal error in DumpToLog().
302+
*
303+
* Example:
304+
* @code
305+
* ChipLogError(Foo, "Delegate=" ChipLogFormatRtti, ChipLogValueRtti(mDelegate));
306+
* @endcode
307+
*/
308+
#define ChipLogFormatRtti "%s"
309+
#if CHIP_HAVE_RTTI
310+
#define ChipLogValueRtti(ptr) ((ptr) != nullptr ? typeid(*(ptr)).name() : "null")
311+
#else
312+
#define ChipLogValueRtti(ptr) ((ptr) != nullptr ? "?" : "null")
313+
#endif
314+
295315
/**
296316
* Logging helpers for protocol ids. A protocol id is a (vendor-id,
297317
* protocol-id) pair.

src/messaging/ExchangeContext.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ ExchangeContext::ExchangeContext(ExchangeManager * em, uint16_t ExchangeId, cons
294294
mDispatch(GetMessageDispatch(isEphemeralExchange, delegate)),
295295
mSession(*this)
296296
{
297-
VerifyOrDie(mExchangeMgr == nullptr);
297+
VerifyOrDieWithObject(mExchangeMgr == nullptr, this);
298298

299299
mExchangeMgr = em;
300300
mExchangeId = ExchangeId;
@@ -334,12 +334,12 @@ ExchangeContext::ExchangeContext(ExchangeManager * em, uint16_t ExchangeId, cons
334334

335335
ExchangeContext::~ExchangeContext()
336336
{
337-
VerifyOrDie(mExchangeMgr != nullptr && GetReferenceCount() == 0);
337+
VerifyOrDieWithObject(mExchangeMgr != nullptr && GetReferenceCount() == 0, this);
338338

339339
//
340340
// Ensure that DoClose has been called by the time we get here. If not, we have a leak somewhere.
341341
//
342-
VerifyOrDie(mFlags.Has(Flags::kFlagClosed));
342+
VerifyOrDieWithObject(mFlags.Has(Flags::kFlagClosed), this);
343343

344344
#if CHIP_CONFIG_ENABLE_ICD_SERVER
345345
// TODO(#33075) : Add check for group context to not a req since it serves no purpose
@@ -666,7 +666,7 @@ void ExchangeContext::AbortAllOtherCommunicationOnFabric()
666666

667667
void ExchangeContext::ExchangeSessionHolder::GrabExpiredSession(const SessionHandle & session)
668668
{
669-
VerifyOrDie(session->AsSecureSession()->IsPendingEviction());
669+
VerifyOrDieWithObject(session->AsSecureSession()->IsPendingEviction(), this);
670670
GrabUnchecked(session);
671671
}
672672

src/messaging/ExchangeContext.h

+7-1
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
158158

159159
SessionHandle GetSessionHandle() const
160160
{
161-
VerifyOrDie(mSession);
161+
VerifyOrDieWithObject(mSession, this);
162162
auto sessionHandle = mSession.Get();
163163
return std::move(sessionHandle.Value());
164164
}
@@ -238,6 +238,12 @@ class DLL_EXPORT ExchangeContext : public ReliableMessageContext,
238238
void ClearInjectedFailures() { mInjectedFailures.ClearAll(); }
239239
#endif
240240

241+
void DumpToLog() const
242+
{
243+
ChipLogError(ExchangeManager, "ExchangeContext: " ChipLogFormatExchangeId " delegate=" ChipLogFormatRtti,
244+
ChipLogValueExchangeId(GetExchangeId(), IsInitiator()), ChipLogValueRtti(mDelegate));
245+
}
246+
241247
private:
242248
#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
243249
BitFlags<InjectedFailureType> mInjectedFailures;

src/messaging/tests/TestExchange.cpp

+12
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <lib/core/StringBuilderAdapters.h>
2424
#include <lib/support/CHIPMem.h>
2525
#include <lib/support/CodeUtils.h>
26+
#include <lib/support/Pool.h>
2627
#include <messaging/ExchangeContext.h>
2728
#include <messaging/ExchangeMgr.h>
2829
#include <messaging/Flags.h>
@@ -219,4 +220,15 @@ TEST_F(TestExchange, CheckBasicExchangeMessageDispatch)
219220
});
220221
}
221222
}
223+
224+
// A crude test to exercise VerifyOrDieWithObject() in ObjectPool and
225+
// the resulting DumpToLog() call on the ExchangeContext.
226+
// TODO: Find a way to automate this test without killing the process.
227+
// TEST_F(TestExchange, DumpExchangePoolToLog)
228+
// {
229+
// MockExchangeDelegate delegate;
230+
// ObjectPool<ExchangeContext, CHIP_CONFIG_MAX_EXCHANGE_CONTEXTS> pool;
231+
// pool.CreateObject(&GetExchangeManager(), static_cast<uint16_t>(1234), GetSessionAliceToBob(), true, &delegate);
232+
// }
233+
222234
} // namespace

0 commit comments

Comments
 (0)