Skip to content

Commit

Permalink
Ensure memory shutdown is done in tests (#15590)
Browse files Browse the repository at this point in the history
* Ensure MemoryInit is paired with MemoryShutdown in tests

* Fix broken logic bits for init/shutdown operations

* Could not fully fix TestCommissioningManager - added a comment for it

* Removed more cleanup logic. It seems pool frees do not work properly in testcommissionermanager

* Remove a redundant Server::Shutdown
  • Loading branch information
andy31415 authored and pull[bot] committed Nov 28, 2023
1 parent 3df71d8 commit 1104770
Show file tree
Hide file tree
Showing 16 changed files with 82 additions and 10 deletions.
5 changes: 5 additions & 0 deletions examples/platform/nrfconnect/util/test/TestInetCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ void InitTestInetCommon()
chip::Platform::MemoryInit();
}

void ShutdownTestInetCommon()
{
chip::Platform::MemoryShutdown();
}

void InitSystemLayer()
{
gSystemLayer.Init();
Expand Down
1 change: 1 addition & 0 deletions src/app/CASESessionManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class CASESessionManager : public Dnssd::OperationalResolveDelegate
virtual ~CASESessionManager() { mDNSResolver.Shutdown(); }

CHIP_ERROR Init();
void Shutdown() { mDNSResolver.Shutdown(); }

/**
* Find an existing session for the given node ID, or trigger a new session request.
Expand Down
1 change: 1 addition & 0 deletions src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ void Server::Shutdown()
mSessions.Shutdown();
mTransports.Close();
mCommissioningWindowManager.Shutdown();
mCASESessionManager.Shutdown();
chip::Platform::MemoryShutdown();
}

Expand Down
25 changes: 23 additions & 2 deletions src/app/tests/TestCommissionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <app/server/CommissioningWindowManager.h>
#include <app/server/Server.h>
#include <lib/dnssd/Advertiser.h>
#include <lib/support/Span.h>
#include <lib/support/UnitTestRegistration.h>
#include <messaging/tests/echo/common.h>
Expand Down Expand Up @@ -52,6 +53,27 @@ void InitializeChip(nlTestSuite * suite)
chip::DeviceLayer::PlatformMgr().StartEventLoopTask();
}

void ShutdownChipTest()
{
chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
chip::DeviceLayer::PlatformMgr().Shutdown();

auto & mdnsAdvertiser = chip::Dnssd::ServiceAdvertiser::Instance();
mdnsAdvertiser.RemoveServices();
mdnsAdvertiser.Shutdown();

// Server shudown will be called in TearDownTask

// TODO: At this point UDP endpoits still seem leaked and the sanitizer
// builds will attempt a memory free. As a result, we keep Memory initialized
// so that the global UDPManager can still be destructed without a coredump.
//
// This is likely either a missing shutdown or an actual UDP endpoint leak
// which I have not been able to track down yet.
//
// chip::Platform::MemoryShutdown();
}

void CheckCommissioningWindowManagerBasicWindowOpenCloseTask(intptr_t context)
{
nlTestSuite * suite = reinterpret_cast<nlTestSuite *>(context);
Expand Down Expand Up @@ -182,8 +204,7 @@ int TestCommissioningWindowManager()
// TODO: The platform memory was intentionally left not deinitialized so that minimal mdns can destruct
chip::DeviceLayer::PlatformMgr().ScheduleWork(TearDownTask, 0);
sleep(kTestTaskWaitSeconds);
chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
chip::DeviceLayer::PlatformMgr().Shutdown();
ShutdownChipTest();

return (nlTestRunnerStats(&theSuite));
}
Expand Down
1 change: 1 addition & 0 deletions src/inet/tests/TestInetCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ extern bool gDone;
void InitTestInetCommon();
void InitSystemLayer();
void ShutdownSystemLayer();
void ShutdownTestInetCommon();
void InetFailError(CHIP_ERROR err, const char * msg);

void InitNetwork();
Expand Down
5 changes: 5 additions & 0 deletions src/inet/tests/TestInetCommonPosix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ void InitTestInetCommon()
UseStdoutLineBuffering();
}

void ShutdownTestInetCommon()
{
chip::Platform::MemoryShutdown();
}

void InitSystemLayer()
{
#if CHIP_SYSTEM_CONFIG_USE_LWIP
Expand Down
2 changes: 2 additions & 0 deletions src/inet/tests/TestInetLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,8 @@ int main(int argc, char * argv[])

lSuccessful = Common::WasSuccessful(sTestState.mStatus);

ShutdownTestInetCommon();

exit:
return (lSuccessful ? EXIT_SUCCESS : EXIT_FAILURE);
}
Expand Down
4 changes: 4 additions & 0 deletions src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,10 @@ int TestAdvertiser(void)
nlTestRunner(&theSuite, &server);
server.Shutdown();
context.Shutdown();
mdnsAdvertiser.RemoveServices();
mdnsAdvertiser.Shutdown();
chip::Platform::MemoryShutdown();

return nlTestRunnerStats(&theSuite);
}

Expand Down
14 changes: 12 additions & 2 deletions src/lib/dnssd/minimal_mdns/tests/TestMinimalMdnsAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -306,12 +306,22 @@ const nlTest sTests[] = {
NL_TEST_SENTINEL() //
};

int TestSetup(void * inContext)
{
return chip::Platform::MemoryInit() == CHIP_NO_ERROR ? SUCCESS : FAILURE;
}

int TestTeardown(void * inContext)
{
chip::Platform::MemoryShutdown();
return SUCCESS;
}

} // namespace

int TestMinimalMdnsAllocator(void)
{
chip::Platform::MemoryInit();
nlTestSuite theSuite = { "MinimalMdnsAllocator", &sTests[0], nullptr, nullptr };
nlTestSuite theSuite = { "MinimalMdnsAllocator", &sTests[0], &TestSetup, &TestTeardown };
nlTestRunner(&theSuite, nullptr);
return nlTestRunnerStats(&theSuite);
}
Expand Down
14 changes: 12 additions & 2 deletions src/lib/dnssd/minimal_mdns/tests/TestResponseSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -354,12 +354,22 @@ const nlTest sTests[] = {
NL_TEST_SENTINEL() //
};

int TestSetup(void * inContext)
{
return chip::Platform::MemoryInit() == CHIP_NO_ERROR ? SUCCESS : FAILURE;
}

int TestTeardown(void * inContext)
{
chip::Platform::MemoryShutdown();
return SUCCESS;
}

} // namespace

int TestResponseSender(void)
{
chip::Platform::MemoryInit();
nlTestSuite theSuite = { "RecordData", sTests, nullptr, nullptr };
nlTestSuite theSuite = { "RecordData", sTests, &TestSetup, &TestTeardown };
nlTestRunner(&theSuite, nullptr);
return nlTestRunnerStats(&theSuite);
}
Expand Down
9 changes: 7 additions & 2 deletions src/lib/dnssd/platform/tests/TestPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,12 @@ void TestCommissionableNode(nlTestSuite * inSuite, void * inContext)

int TestSetup(void * inContext)
{
chip::Platform::MemoryInit();
return chip::Platform::MemoryInit() == CHIP_NO_ERROR ? SUCCESS : FAILURE;
}

int TestTeardown(void * inContext)
{
chip::Platform::MemoryShutdown();
return SUCCESS;
}

Expand All @@ -236,7 +241,7 @@ const nlTest sTests[] = {

int TestDnssdPlatform(void)
{
nlTestSuite theSuite = { "DnssdPlatform", &sTests[0], &TestSetup, nullptr };
nlTestSuite theSuite = { "DnssdPlatform", &sTests[0], &TestSetup, &TestTeardown };
nlTestRunner(&theSuite, nullptr);
return nlTestRunnerStats(&theSuite);
}
Expand Down
2 changes: 2 additions & 0 deletions src/lib/support/tests/TestCHIPArgParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,8 @@ int TestCHIPArgParser(void)

printf("All tests succeeded\n");

chip::Platform::MemoryShutdown();

return (EXIT_SUCCESS);
}
#else // CHIP_CONFIG_ENABLE_ARG_PARSER
Expand Down
4 changes: 2 additions & 2 deletions src/platform/tests/TestCHIPoBLEStackMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ int TestCHIPoBLEStackManager()
ChipLogProgress(DeviceLayer, "Start Chip Over Ble stack Done");

chip::DeviceLayer::PlatformMgrImpl().RunEventLoop();
ChipLogProgress(DeviceLayer, "Start EventLoop");

ChipLogProgress(DeviceLayer, "RunEventLoop completed");
chip::Platform::MemoryShutdown();
return 0;
}

Expand Down
2 changes: 2 additions & 0 deletions src/platform/tests/TestThreadStackMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ void EventHandler(const chip::DeviceLayer::ChipDeviceEvent * event, intptr_t arg
{
if (event->ThreadConnectivityChange.Result == chip::DeviceLayer::ConnectivityChange::kConnectivity_Established)
{
chip::Platform::MemoryShutdown();
exit(0);
}
}
Expand Down Expand Up @@ -76,6 +77,7 @@ int TestThreadStackManager()
printf("Start Thread task done\n");

chip::DeviceLayer::PlatformMgrImpl().RunEventLoop();
chip::Platform::MemoryShutdown();

return -1;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ int Finalize(void * aContext)
{
CHIP_ERROR err = reinterpret_cast<TestContext *>(aContext)->Shutdown();
gIOContext.Shutdown();
chip::Platform::MemoryShutdown();
return (err == CHIP_NO_ERROR) ? SUCCESS : FAILURE;
}

Expand Down
2 changes: 2 additions & 0 deletions src/system/tests/TestSystemPacketBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ int PacketBufferTest::TestTeardown(void * inContext)
if (err != CHIP_NO_ERROR && err != CHIP_ERROR_NOT_IMPLEMENTED)
return FAILURE;

chip::Platform::MemoryShutdown();

return SUCCESS;
}

Expand Down

0 comments on commit 1104770

Please sign in to comment.