Skip to content

Commit

Permalink
Fix CommissioningWindowManager DNS UDP leak (#11983)
Browse files Browse the repository at this point in the history
#### Problem

`chip::Server::Shutdown()` contains

```
chip::Dnssd::ServiceAdvertiser::Instance().Shutdown();
    ⋮
mCommissioningWindowManager.Cleanup();
```

but the latter restarts `Dnssd::ServiceAdvertiser`.

Instance of #11880 _Possible use of destroyed pool objects_

#### Change overview

Add `CommissioningWindowManager::Shutdown()` which does not restart DNS.

#### Testing

If `ObjectPool` checks that objects do not outlive it
(originally part of PR #11698 but deferred due to current leaks),
then `TestCommissionManager` fails without this change.
  • Loading branch information
kpschoedel authored and pull[bot] committed Jan 20, 2023
1 parent c12484e commit 1534492
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 3 deletions.
7 changes: 6 additions & 1 deletion src/app/server/CommissioningWindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ void CommissioningWindowManager::OnPlatformEvent(const DeviceLayer::ChipDeviceEv
}
}

void CommissioningWindowManager::Cleanup()
void CommissioningWindowManager::Shutdown()
{
StopAdvertisement();

Expand All @@ -84,6 +84,11 @@ void CommissioningWindowManager::Cleanup()

memset(&mECMPASEVerifier, 0, sizeof(mECMPASEVerifier));
memset(mECMSalt, 0, sizeof(mECMSalt));
}

void CommissioningWindowManager::Cleanup()
{
Shutdown();

// reset all advertising
app::DnssdServer::Instance().StartServer(Dnssd::CommissioningMode::kDisabled);
Expand Down
1 change: 1 addition & 0 deletions src/app/server/CommissioningWindowManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class CommissioningWindowManager : public SessionEstablishmentDelegate
void OnSessionEstablishmentStarted() override;
void OnSessionEstablished() override;

void Shutdown();
void Cleanup();

void OnPlatformEvent(const DeviceLayer::ChipDeviceEvent * event);
Expand Down
2 changes: 1 addition & 1 deletion src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ void Server::Shutdown()
mExchangeMgr.Shutdown();
mSessions.Shutdown();
mTransports.Close();
mCommissioningWindowManager.Cleanup();
mCommissioningWindowManager.Shutdown();
chip::Platform::MemoryShutdown();
}

Expand Down
3 changes: 2 additions & 1 deletion src/app/tests/TestCommissionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ void CheckCommissioningWindowManagerEnhancedWindow(nlTestSuite * suite, void *)
void TearDownTask(intptr_t context)
{
chip::Server::GetInstance().Shutdown();
chip::DeviceLayer::PlatformMgr().Shutdown();
}

const nlTest sTests[] = {
Expand Down Expand Up @@ -167,6 +166,8 @@ 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();

return (nlTestRunnerStats(&theSuite));
}
Expand Down

0 comments on commit 1534492

Please sign in to comment.