From 62131e0b974883771def22dc653f651cefc52507 Mon Sep 17 00:00:00 2001 From: Kevin Schoedel Date: Wed, 18 Aug 2021 12:43:17 -0400 Subject: [PATCH 1/2] Add ChipMdnsShutdown() #### Problem Issue #8001 MdnsAvahi destructor segfault; require separate shutdown MdnsAvahi can cause a crash on exit due to a static instance running code in its destructor after dependent components have shut down. #### Change overview - Add `ChipMdnsShutdown()`. This is a narrow fix, which does not attempt to address other potential problems with singleton mDNS state. - Fix TestMdns to shut down cleanly. #### Testing - TestMdns and CI. --- src/controller/java/MdnsImpl.cpp | 5 +++++ src/lib/mdns/platform/Mdns.h | 11 +++++++++- src/platform/Darwin/MdnsImpl.cpp | 5 +++++ src/platform/ESP32/MdnsImpl.cpp | 5 +++++ src/platform/Linux/MdnsImpl.cpp | 33 ++++++++++++++++++---------- src/platform/Linux/MdnsImpl.h | 3 +-- src/platform/OpenThread/MdnsImpl.cpp | 5 +++++ src/platform/tests/TestMdns.cpp | 15 +++++++++++++ 8 files changed, 67 insertions(+), 15 deletions(-) diff --git a/src/controller/java/MdnsImpl.cpp b/src/controller/java/MdnsImpl.cpp index 5e8eafb2dc42e2..86d3fc4570bc0c 100644 --- a/src/controller/java/MdnsImpl.cpp +++ b/src/controller/java/MdnsImpl.cpp @@ -47,6 +47,11 @@ CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCal return CHIP_NO_ERROR; } +CHIP_ERROR ChipMdnsShutdown() +{ + return CHIP_NO_ERROR; +} + CHIP_ERROR ChipMdnsPublishService(const MdnsService * service) { return CHIP_ERROR_NOT_IMPLEMENTED; diff --git a/src/lib/mdns/platform/Mdns.h b/src/lib/mdns/platform/Mdns.h index af9b9dc53aec3a..09739a16ef7909 100644 --- a/src/lib/mdns/platform/Mdns.h +++ b/src/lib/mdns/platform/Mdns.h @@ -107,7 +107,7 @@ using MdnsBrowseCallback = void (*)(void * context, MdnsService * services, size using MdnsAsyncReturnCallback = void (*)(void * context, CHIP_ERROR error); /** - * This function intializes the mdns module + * This function initializes the mdns module * * @param[in] initCallback The callback for notifying the initialization result. * @param[in] errorCallback The callback for notifying internal errors. @@ -119,6 +119,15 @@ using MdnsAsyncReturnCallback = void (*)(void * context, CHIP_ERROR error); */ CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCallback errorCallback, void * context); +/** + * This function shuts down the mdns module + * + * @retval CHIP_NO_ERROR The shutdown succeeds. + * @retval Error code The shutdown fails + * + */ +CHIP_ERROR ChipMdnsShutdown(); + /** * This function publishes an service via mDNS. * diff --git a/src/platform/Darwin/MdnsImpl.cpp b/src/platform/Darwin/MdnsImpl.cpp index 76fa6a1b538cbb..284eaeb8e4c974 100644 --- a/src/platform/Darwin/MdnsImpl.cpp +++ b/src/platform/Darwin/MdnsImpl.cpp @@ -485,6 +485,11 @@ CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback successCallback, MdnsAsyncReturn return CHIP_NO_ERROR; } +CHIP_ERROR ChipMdnsShutdown() +{ + return CHIP_NO_ERROR; +} + CHIP_ERROR ChipMdnsPublishService(const MdnsService * service) { VerifyOrReturnError(service != nullptr, CHIP_ERROR_INVALID_ARGUMENT); diff --git a/src/platform/ESP32/MdnsImpl.cpp b/src/platform/ESP32/MdnsImpl.cpp index edc9ea1faa3f68..15c15a2112d993 100644 --- a/src/platform/ESP32/MdnsImpl.cpp +++ b/src/platform/ESP32/MdnsImpl.cpp @@ -55,6 +55,11 @@ CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCal return error; } +CHIP_ERROR ChipMdnsShutdown() +{ + return CHIP_NO_ERROR; +} + static const char * GetProtocolString(MdnsServiceProtocol protocol) { return protocol == MdnsServiceProtocol::kMdnsProtocolTcp ? "_tcp" : "_udp"; diff --git a/src/platform/Linux/MdnsImpl.cpp b/src/platform/Linux/MdnsImpl.cpp index 3ac48d30eeb57a..69b81c93853252 100644 --- a/src/platform/Linux/MdnsImpl.cpp +++ b/src/platform/Linux/MdnsImpl.cpp @@ -338,6 +338,22 @@ CHIP_ERROR MdnsAvahi::Init(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturn return error; } + +CHIP_ERROR MdnsAvahi::Shutdown() +{ + if (mGroup) + { + avahi_entry_group_free(mGroup); + mGroup = nullptr; + } + if (mClient) + { + avahi_client_free(mClient); + mClient = nullptr; + } + return CHIP_NO_ERROR; +} + CHIP_ERROR MdnsAvahi::SetHostname(const char * hostname) { CHIP_ERROR error = CHIP_NO_ERROR; @@ -733,18 +749,6 @@ void MdnsAvahi::HandleResolve(AvahiServiceResolver * resolver, AvahiIfIndex inte chip::Platform::Delete(context); } -MdnsAvahi::~MdnsAvahi() -{ - if (mGroup) - { - avahi_entry_group_free(mGroup); - } - if (mClient) - { - avahi_client_free(mClient); - } -} - void GetMdnsTimeout(timeval & timeout) { MdnsAvahi::GetInstance().GetPoller().GetTimeout(timeout); @@ -760,6 +764,11 @@ CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCal return MdnsAvahi::GetInstance().Init(initCallback, errorCallback, context); } +CHIP_ERROR ChipMdnsShutdown() +{ + return MdnsAvahi::GetInstance().Shutdown(); +} + CHIP_ERROR ChipMdnsPublishService(const MdnsService * service) { if (strcmp(service->mHostName, "") != 0) diff --git a/src/platform/Linux/MdnsImpl.h b/src/platform/Linux/MdnsImpl.h index 7360c148b6ed4b..db8a244fa086b7 100644 --- a/src/platform/Linux/MdnsImpl.h +++ b/src/platform/Linux/MdnsImpl.h @@ -100,6 +100,7 @@ class MdnsAvahi MdnsAvahi & operator=(const MdnsAvahi &) = delete; CHIP_ERROR Init(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCallback errorCallback, void * context); + CHIP_ERROR Shutdown(); CHIP_ERROR SetHostname(const char * hostname); CHIP_ERROR PublishService(const MdnsService & service); CHIP_ERROR StopPublish(); @@ -112,8 +113,6 @@ class MdnsAvahi static MdnsAvahi & GetInstance() { return sInstance; } - ~MdnsAvahi(); - private: struct BrowseContext { diff --git a/src/platform/OpenThread/MdnsImpl.cpp b/src/platform/OpenThread/MdnsImpl.cpp index 05a5b0ba306bf9..b407e7a5736a12 100644 --- a/src/platform/OpenThread/MdnsImpl.cpp +++ b/src/platform/OpenThread/MdnsImpl.cpp @@ -32,6 +32,11 @@ CHIP_ERROR ChipMdnsInit(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturnCal return CHIP_NO_ERROR; } +CHIP_ERROR ChipMdnsShutdown() +{ + return CHIP_NO_ERROR; +} + const char * GetProtocolString(MdnsServiceProtocol protocol) { return protocol == MdnsServiceProtocol::kMdnsProtocolUdp ? "_udp" : "_tcp"; diff --git a/src/platform/tests/TestMdns.cpp b/src/platform/tests/TestMdns.cpp index 55bdd7f33abc2a..5f5dd602b83d56 100644 --- a/src/platform/tests/TestMdns.cpp +++ b/src/platform/tests/TestMdns.cpp @@ -27,6 +27,8 @@ static void HandleResolve(void * context, MdnsService * result, CHIP_ERROR error NL_TEST_ASSERT(suite, strcmp(result->mTextEntries[0].mKey, "key") == 0); NL_TEST_ASSERT(suite, strcmp(reinterpret_cast(result->mTextEntries[0].mData), "val") == 0); + chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); + chip::DeviceLayer::PlatformMgr().Shutdown(); exit(0); } @@ -104,6 +106,7 @@ int TestMdns() std::condition_variable doneCondition; bool done = false; + bool shutdown = false; int retVal = EXIT_FAILURE; @@ -139,8 +142,10 @@ int TestMdns() // This will stop the event loop above, and wait till it has actually stopped // (i.e exited RunEventLoop()). // + chip::Mdns::ChipMdnsShutdown(); chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); chip::DeviceLayer::PlatformMgr().Shutdown(); + shutdown = true; doneCondition.wait_for(lock, std::chrono::seconds(1)); if (!done) @@ -150,6 +155,16 @@ int TestMdns() retVal = EXIT_FAILURE; } } + t.join(); + + if (!shutdown) + { + chip::Mdns::ChipMdnsShutdown(); + chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); + chip::DeviceLayer::PlatformMgr().Shutdown(); + } + chip::Platform::MemoryShutdown(); + return retVal; } From 2bddacb38195d2ee8d6ebcc9b39189f36150ac4c Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Wed, 18 Aug 2021 18:28:34 +0000 Subject: [PATCH 2/2] Restyled by clang-format --- src/platform/Linux/MdnsImpl.cpp | 1 - src/platform/tests/TestMdns.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/platform/Linux/MdnsImpl.cpp b/src/platform/Linux/MdnsImpl.cpp index 69b81c93853252..c8725a5dcdfd42 100644 --- a/src/platform/Linux/MdnsImpl.cpp +++ b/src/platform/Linux/MdnsImpl.cpp @@ -338,7 +338,6 @@ CHIP_ERROR MdnsAvahi::Init(MdnsAsyncReturnCallback initCallback, MdnsAsyncReturn return error; } - CHIP_ERROR MdnsAvahi::Shutdown() { if (mGroup) diff --git a/src/platform/tests/TestMdns.cpp b/src/platform/tests/TestMdns.cpp index 5f5dd602b83d56..388f1dd63e61e5 100644 --- a/src/platform/tests/TestMdns.cpp +++ b/src/platform/tests/TestMdns.cpp @@ -105,7 +105,7 @@ int TestMdns() bool ready = false; std::condition_variable doneCondition; - bool done = false; + bool done = false; bool shutdown = false; int retVal = EXIT_FAILURE;