Skip to content

Commit

Permalink
Add a way to avoid heap pools crashing on exit if items are still liv…
Browse files Browse the repository at this point in the history
…e. (#23359)

Currently a pool's destructor crashes if there are still live objects in the
pool.  For heap pools, add a way to not crash if the pool's destructor is
running under exit(), for consumers that don't care about exit-time-only leaks.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jun 20, 2023
1 parent b783005 commit 1307034
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 3 deletions.
6 changes: 6 additions & 0 deletions src/darwin/Framework/CHIP/MTRControllerFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include <credentials/attestation_verifier/DefaultDeviceAttestationVerifier.h>
#include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
#include <crypto/PersistentStorageOperationalKeystore.h>
#include <lib/support/Pool.h>
#include <lib/support/TestPersistentStorageDelegate.h>
#include <platform/PlatformManager.h>

Expand Down Expand Up @@ -309,6 +310,11 @@ - (BOOL)startup:(MTRControllerFactoryParams *)startupParams
return;
}

// This needs to happen after DeviceControllerFactory::Init,
// because that creates (lazily, by calling functions with
// static variables in them) some static-lifetime objects.
chip::HeapObjectPoolExitHandling::IgnoreLeaksOnExit();

// Make sure we don't leave a system state running while we have no
// controllers started. This is working around the fact that a system
// state is brought up live on factory init, and not when it comes time
Expand Down
27 changes: 27 additions & 0 deletions src/lib/support/Pool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,33 @@

namespace chip {

#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

bool HeapObjectPoolExitHandling::sIgnoringLeaksOnExit = false;
bool HeapObjectPoolExitHandling::sExitHandlerRegistered = false;

void HeapObjectPoolExitHandling::IgnoreLeaksOnExit()
{
if (sExitHandlerRegistered)
{
return;
}

int ret = atexit(ExitHandler);
if (ret != 0)
{
ChipLogError(Controller, "IgnoreLeaksOnExit: atexit failed: %d\n", ret);
}
sExitHandlerRegistered = true;
}

void HeapObjectPoolExitHandling::ExitHandler()
{
sIgnoringLeaksOnExit = true;
}

#endif // CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

namespace internal {

StaticAllocatorBitmap::StaticAllocatorBitmap(void * storage, std::atomic<tBitChunkType> * usage, size_t capacity,
Expand Down
26 changes: 23 additions & 3 deletions src/lib/support/Pool.h
Original file line number Diff line number Diff line change
Expand Up @@ -291,13 +291,30 @@ class BitMapObjectPool : public internal::StaticAllocatorBitmap, public internal

#if CHIP_SYSTEM_CONFIG_POOL_USE_HEAP

class HeapObjectPoolExitHandling
{
public:
// If IgnoreLeaksOnExit is called, some time after all static initializers have
// run, HeapObjectPool will not assert that everything in it has been
// released if its destructor runs under exit() (i.e. when the application
// is quitting anyway).
static void IgnoreLeaksOnExit();

protected:
static bool sIgnoringLeaksOnExit;

private:
static void ExitHandler();
static bool sExitHandlerRegistered;
};

/**
* A class template used for allocating objects from the heap.
*
* @tparam T type to be allocated.
*/
template <class T>
class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<T>
class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<T>, public HeapObjectPoolExitHandling
{
public:
HeapObjectPool() {}
Expand All @@ -314,8 +331,11 @@ class HeapObjectPool : public internal::Statistics, public internal::PoolCommon<
// Free all remaining objects so that ASAN can catch specific use-after-free cases.
ReleaseAll();
#else // __SANITIZE_ADDRESS__
// Verify that no live objects remain, to prevent potential use-after-free.
VerifyOrDie(Allocated() == 0);
if (!sIgnoringLeaksOnExit)
{
// Verify that no live objects remain, to prevent potential use-after-free.
VerifyOrDie(Allocated() == 0);
}
#endif // __SANITIZE_ADDRESS__
}

Expand Down

0 comments on commit 1307034

Please sign in to comment.