Skip to content

Commit

Permalink
Add ChipMdnsShutdown() (#9108)
Browse files Browse the repository at this point in the history
* 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.

* restyle
  • Loading branch information
kpschoedel authored Aug 19, 2021
1 parent 43bd2b6 commit 09916d7
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 16 deletions.
5 changes: 5 additions & 0 deletions src/controller/java/MdnsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 10 additions & 1 deletion src/lib/mdns/platform/Mdns.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
*
Expand Down
5 changes: 5 additions & 0 deletions src/platform/Darwin/MdnsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions src/platform/ESP32/MdnsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
32 changes: 20 additions & 12 deletions src/platform/Linux/MdnsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,21 @@ 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;
Expand Down Expand Up @@ -733,18 +748,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);
Expand All @@ -760,6 +763,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)
Expand Down
3 changes: 1 addition & 2 deletions src/platform/Linux/MdnsImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -112,8 +113,6 @@ class MdnsAvahi

static MdnsAvahi & GetInstance() { return sInstance; }

~MdnsAvahi();

private:
struct BrowseContext
{
Expand Down
5 changes: 5 additions & 0 deletions src/platform/OpenThread/MdnsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
17 changes: 16 additions & 1 deletion src/platform/tests/TestMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<const char *>(result->mTextEntries[0].mData), "val") == 0);

chip::DeviceLayer::PlatformMgr().StopEventLoopTask();
chip::DeviceLayer::PlatformMgr().Shutdown();
exit(0);
}

Expand Down Expand Up @@ -103,7 +105,8 @@ int TestMdns()
bool ready = false;

std::condition_variable doneCondition;
bool done = false;
bool done = false;
bool shutdown = false;

int retVal = EXIT_FAILURE;

Expand Down Expand Up @@ -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)
Expand All @@ -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;
}

Expand Down

0 comments on commit 09916d7

Please sign in to comment.