From cbf5e3e04454a609b8c8ab859f8612b6ec428a62 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 18 Apr 2023 10:51:50 +0200 Subject: [PATCH 01/11] Fix TestDnssd exit after last HandleResolved call --- src/platform/tests/TestDnssd.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/tests/TestDnssd.cpp b/src/platform/tests/TestDnssd.cpp index 4a0437ce35bfb6..71540355d32355 100644 --- a/src/platform/tests/TestDnssd.cpp +++ b/src/platform/tests/TestDnssd.cpp @@ -39,9 +39,9 @@ static void HandleResolve(void * context, DnssdService * result, const chip::Spa if (gBrowsedServicesCount == ++gResolvedServicesCount) { + // After last service is resolved, stop the event loop, + // so the test case can gracefully exit. chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); - chip::DeviceLayer::PlatformMgr().Shutdown(); - exit(0); } } From 7c98d3cf634d845ad6924a2a165809e59b1f2d46 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 25 Apr 2023 16:08:04 +0200 Subject: [PATCH 02/11] Fix typo in comment --- src/platform/tests/TestConfigurationMgr.cpp | 2 +- src/platform/tests/TestConnectivityMgr.cpp | 2 +- src/platform/tests/TestDnssd.cpp | 23 +++++++++++++++++++++ src/platform/tests/TestKeyValueStoreMgr.cpp | 2 +- src/platform/tests/TestPlatformMgr.cpp | 2 +- src/platform/tests/TestPlatformTime.cpp | 2 +- 6 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/platform/tests/TestConfigurationMgr.cpp b/src/platform/tests/TestConfigurationMgr.cpp index aafdf4a63cfa31..91dabe222f28fd 100644 --- a/src/platform/tests/TestConfigurationMgr.cpp +++ b/src/platform/tests/TestConfigurationMgr.cpp @@ -499,7 +499,7 @@ int TestConfigurationMgr() { nlTestSuite theSuite = { "ConfigurationMgr tests", &sTests[0], TestConfigurationMgr_Setup, TestConfigurationMgr_Teardown }; - // Run test suit againt one context. + // Run test suit against one context. nlTestRunner(&theSuite, nullptr); return nlTestRunnerStats(&theSuite); } diff --git a/src/platform/tests/TestConnectivityMgr.cpp b/src/platform/tests/TestConnectivityMgr.cpp index 932be3dcfc5b1a..6cf205737a330a 100644 --- a/src/platform/tests/TestConnectivityMgr.cpp +++ b/src/platform/tests/TestConnectivityMgr.cpp @@ -99,7 +99,7 @@ int TestConnectivityMgr() { nlTestSuite theSuite = { "ConfigurationMgr tests", &sTests[0], TestConnectivityMgr_Setup, TestConnectivityMgr_Teardown }; - // Run test suit againt one context. + // Run test suit against one context. nlTestRunner(&theSuite, nullptr); return nlTestRunnerStats(&theSuite); } diff --git a/src/platform/tests/TestDnssd.cpp b/src/platform/tests/TestDnssd.cpp index 71540355d32355..f6cbb22e202be2 100644 --- a/src/platform/tests/TestDnssd.cpp +++ b/src/platform/tests/TestDnssd.cpp @@ -1,3 +1,26 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file + * This file implements a unit test suite for the mDNS code functionality. + * + */ + #include #include #include diff --git a/src/platform/tests/TestKeyValueStoreMgr.cpp b/src/platform/tests/TestKeyValueStoreMgr.cpp index fad7e490078069..f189493afe6956 100644 --- a/src/platform/tests/TestKeyValueStoreMgr.cpp +++ b/src/platform/tests/TestKeyValueStoreMgr.cpp @@ -353,7 +353,7 @@ int TestKeyValueStoreMgr() { nlTestSuite theSuite = { "KeyValueStoreMgr tests", &sTests[0], TestKeyValueStoreMgr_Setup, TestKeyValueStoreMgr_Teardown }; - // Run test suit againt one context. + // Run test suit against one context. nlTestRunner(&theSuite, nullptr); return nlTestRunnerStats(&theSuite); } diff --git a/src/platform/tests/TestPlatformMgr.cpp b/src/platform/tests/TestPlatformMgr.cpp index cd2caab4b96eb9..1e8b5741c70369 100644 --- a/src/platform/tests/TestPlatformMgr.cpp +++ b/src/platform/tests/TestPlatformMgr.cpp @@ -247,7 +247,7 @@ int TestPlatformMgr() { nlTestSuite theSuite = { "PlatformMgr tests", &sTests[0], TestPlatformMgr_Setup, TestPlatformMgr_Teardown }; - // Run test suit againt one context. + // Run test suit against one context. nlTestRunner(&theSuite, nullptr); return nlTestRunnerStats(&theSuite); } diff --git a/src/platform/tests/TestPlatformTime.cpp b/src/platform/tests/TestPlatformTime.cpp index 95698f0d696f15..cca7db7e69c691 100644 --- a/src/platform/tests/TestPlatformTime.cpp +++ b/src/platform/tests/TestPlatformTime.cpp @@ -125,7 +125,7 @@ int TestPlatformTime() { nlTestSuite theSuite = { "PlatformTime tests", &sTests[0], nullptr, nullptr }; - // Run test suit againt one context. + // Run test suit against one context. nlTestRunner(&theSuite, nullptr); return nlTestRunnerStats(&theSuite); } From 5595a24846073d4988c4fe32720d3476bd9eb169 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 25 Apr 2023 16:11:59 +0200 Subject: [PATCH 03/11] Use dedicated context instead of global state --- src/platform/tests/TestDnssd.cpp | 49 +++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/platform/tests/TestDnssd.cpp b/src/platform/tests/TestDnssd.cpp index f6cbb22e202be2..1b2fe5078b58de 100644 --- a/src/platform/tests/TestDnssd.cpp +++ b/src/platform/tests/TestDnssd.cpp @@ -37,15 +37,25 @@ using chip::Dnssd::DnssdService; using chip::Dnssd::DnssdServiceProtocol; using chip::Dnssd::TextEntry; -static unsigned int gBrowsedServicesCount = 0; -static unsigned int gResolvedServicesCount = 0; -static bool gEndOfInput = false; +namespace { + +struct DnssdContext +{ + nlTestSuite * mTestSuite; + + unsigned int mBrowsedServicesCount = 0; + unsigned int mResolvedServicesCount = 0; + bool mEndOfInput = false; +}; + +} // namespace static void HandleResolve(void * context, DnssdService * result, const chip::Span & addresses, CHIP_ERROR error) { + auto * ctx = static_cast(context); + auto * suite = ctx->mTestSuite; char addrBuf[100]; - nlTestSuite * suite = static_cast(context); NL_TEST_ASSERT(suite, result != nullptr); NL_TEST_ASSERT(suite, error == CHIP_NO_ERROR); @@ -53,14 +63,14 @@ static void HandleResolve(void * context, DnssdService * result, const chip::Spa if (!addresses.empty()) { addresses.data()[0].ToString(addrBuf, sizeof(addrBuf)); - printf("Service[%u] at [%s]:%u\n", gResolvedServicesCount, addrBuf, result->mPort); + printf("Service[%u] at [%s]:%u\n", ctx->mResolvedServicesCount, addrBuf, result->mPort); } NL_TEST_ASSERT(suite, result->mTextEntrySize == 1); NL_TEST_ASSERT(suite, strcmp(result->mTextEntries[0].mKey, "key") == 0); NL_TEST_ASSERT(suite, strcmp(reinterpret_cast(result->mTextEntries[0].mData), "val") == 0); - if (gBrowsedServicesCount == ++gResolvedServicesCount) + if (ctx->mBrowsedServicesCount == ++ctx->mResolvedServicesCount) { // After last service is resolved, stop the event loop, // so the test case can gracefully exit. @@ -70,14 +80,15 @@ static void HandleResolve(void * context, DnssdService * result, const chip::Spa static void HandleBrowse(void * context, DnssdService * services, size_t servicesSize, bool finalBrowse, CHIP_ERROR error) { - nlTestSuite * suite = static_cast(context); + auto * ctx = static_cast(context); + auto * suite = ctx->mTestSuite; // Make sure that we will not be called again after end-of-input is set - NL_TEST_ASSERT(suite, gEndOfInput == false); + NL_TEST_ASSERT(suite, ctx->mEndOfInput == false); NL_TEST_ASSERT(suite, error == CHIP_NO_ERROR); - gBrowsedServicesCount += servicesSize; - gEndOfInput = finalBrowse; + ctx->mBrowsedServicesCount += servicesSize; + ctx->mEndOfInput = finalBrowse; if (servicesSize > 0) { @@ -86,7 +97,7 @@ static void HandleBrowse(void * context, DnssdService * services, size_t service { printf("Service[%u] name %s\n", i, services[i].mName); printf("Service[%u] type %s\n", i, services[i].mType); - NL_TEST_ASSERT(suite, ChipDnssdResolve(&services[i], services[i].mInterface, HandleResolve, suite) == CHIP_NO_ERROR); + NL_TEST_ASSERT(suite, ChipDnssdResolve(&services[i], services[i].mInterface, HandleResolve, context) == CHIP_NO_ERROR); } } } @@ -95,11 +106,13 @@ static void HandlePublish(void * context, const char * type, const char * instan static void InitCallback(void * context, CHIP_ERROR error) { + auto * ctx = static_cast(context); + auto * suite = ctx->mTestSuite; + DnssdService service; TextEntry entry; - char key[] = "key"; - char val[] = "val"; - nlTestSuite * suite = static_cast(context); + char key[] = "key"; + char val[] = "val"; NL_TEST_ASSERT(suite, error == CHIP_NO_ERROR); @@ -119,9 +132,10 @@ static void InitCallback(void * context, CHIP_ERROR error) service.mSubTypeSize = 0; NL_TEST_ASSERT(suite, ChipDnssdPublishService(&service, HandlePublish) == CHIP_NO_ERROR); + intptr_t browseIdentifier; ChipDnssdBrowse("_mock", DnssdServiceProtocol::kDnssdProtocolTcp, chip::Inet::IPAddressType::kAny, - chip::Inet::InterfaceId::Null(), HandleBrowse, suite, &browseIdentifier); + chip::Inet::InterfaceId::Null(), HandleBrowse, context, &browseIdentifier); } static void ErrorCallback(void * context, CHIP_ERROR error) @@ -131,9 +145,12 @@ static void ErrorCallback(void * context, CHIP_ERROR error) void TestDnssdPubSub(nlTestSuite * inSuite, void * inContext) { + DnssdContext context; + context.mTestSuite = inSuite; + chip::Platform::MemoryInit(); chip::DeviceLayer::PlatformMgr().InitChipStack(); - NL_TEST_ASSERT(inSuite, chip::Dnssd::ChipDnssdInit(InitCallback, ErrorCallback, inSuite) == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, chip::Dnssd::ChipDnssdInit(InitCallback, ErrorCallback, &context) == CHIP_NO_ERROR); ChipLogProgress(DeviceLayer, "Start EventLoop"); chip::DeviceLayer::PlatformMgr().RunEventLoop(); From 8dafff9ed71a3608a4c4b331c82364fdbcf08905 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 25 Apr 2023 16:28:17 +0200 Subject: [PATCH 04/11] Use Setup/Teardown for DNS-SD test suite --- src/platform/tests/TestDnssd.cpp | 129 ++++++++++--------------------- 1 file changed, 41 insertions(+), 88 deletions(-) diff --git a/src/platform/tests/TestDnssd.cpp b/src/platform/tests/TestDnssd.cpp index 1b2fe5078b58de..a5b3407e331b62 100644 --- a/src/platform/tests/TestDnssd.cpp +++ b/src/platform/tests/TestDnssd.cpp @@ -102,19 +102,26 @@ static void HandleBrowse(void * context, DnssdService * services, size_t service } } -static void HandlePublish(void * context, const char * type, const char * instanceName, CHIP_ERROR error) {} +static void DnssdErrorCallback(void * context, CHIP_ERROR error) +{ + auto * ctx = static_cast(context); + NL_TEST_ASSERT(ctx->mTestSuite, error == CHIP_NO_ERROR); +} -static void InitCallback(void * context, CHIP_ERROR error) +static void HandlePublish(void * context, const char * type, const char * instanceName, CHIP_ERROR error) { - auto * ctx = static_cast(context); - auto * suite = ctx->mTestSuite; + auto * ctx = static_cast(context); + NL_TEST_ASSERT(ctx->mTestSuite, error == CHIP_NO_ERROR); +} - DnssdService service; - TextEntry entry; - char key[] = "key"; - char val[] = "val"; +static void TestDnssdPubSub_DnssdInitCallback(void * context, CHIP_ERROR error) +{ + auto * ctx = static_cast(context); + NL_TEST_ASSERT(ctx->mTestSuite, error == CHIP_NO_ERROR); + auto * suite = ctx->mTestSuite; - NL_TEST_ASSERT(suite, error == CHIP_NO_ERROR); + DnssdService service{}; + TextEntry entry{ "key", reinterpret_cast("val"), 3 }; service.mInterface = chip::Inet::InterfaceId::Null(); service.mPort = 80; @@ -123,24 +130,17 @@ static void InitCallback(void * context, CHIP_ERROR error) strcpy(service.mType, "_mock"); service.mAddressType = chip::Inet::IPAddressType::kAny; service.mProtocol = DnssdServiceProtocol::kDnssdProtocolTcp; - entry.mKey = key; - entry.mData = reinterpret_cast(val); - entry.mDataSize = strlen(reinterpret_cast(entry.mData)); service.mTextEntries = &entry; service.mTextEntrySize = 1; service.mSubTypes = nullptr; service.mSubTypeSize = 0; - NL_TEST_ASSERT(suite, ChipDnssdPublishService(&service, HandlePublish) == CHIP_NO_ERROR); + NL_TEST_ASSERT(suite, ChipDnssdPublishService(&service, HandlePublish, context) == CHIP_NO_ERROR); intptr_t browseIdentifier; - ChipDnssdBrowse("_mock", DnssdServiceProtocol::kDnssdProtocolTcp, chip::Inet::IPAddressType::kAny, - chip::Inet::InterfaceId::Null(), HandleBrowse, context, &browseIdentifier); -} - -static void ErrorCallback(void * context, CHIP_ERROR error) -{ - VerifyOrDieWithMsg(error == CHIP_NO_ERROR, DeviceLayer, "Mdns error: %" CHIP_ERROR_FORMAT "\n", error.Format()); + NL_TEST_ASSERT(suite, + ChipDnssdBrowse("_mock", DnssdServiceProtocol::kDnssdProtocolTcp, chip::Inet::IPAddressType::kAny, + chip::Inet::InterfaceId::Null(), HandleBrowse, context, &browseIdentifier) == CHIP_NO_ERROR); } void TestDnssdPubSub(nlTestSuite * inSuite, void * inContext) @@ -148,86 +148,39 @@ void TestDnssdPubSub(nlTestSuite * inSuite, void * inContext) DnssdContext context; context.mTestSuite = inSuite; - chip::Platform::MemoryInit(); - chip::DeviceLayer::PlatformMgr().InitChipStack(); - NL_TEST_ASSERT(inSuite, chip::Dnssd::ChipDnssdInit(InitCallback, ErrorCallback, &context) == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, + chip::Dnssd::ChipDnssdInit(TestDnssdPubSub_DnssdInitCallback, DnssdErrorCallback, &context) == CHIP_NO_ERROR); ChipLogProgress(DeviceLayer, "Start EventLoop"); chip::DeviceLayer::PlatformMgr().RunEventLoop(); ChipLogProgress(DeviceLayer, "End EventLoop"); + + chip::Dnssd::ChipDnssdShutdown(); } static const nlTest sTests[] = { NL_TEST_DEF("Test Dnssd::PubSub", TestDnssdPubSub), NL_TEST_SENTINEL() }; -int TestDnssd() +int TestDnssd_Setup(void * inContext) { - std::mutex mtx; - - std::condition_variable readyCondition; - bool ready = false; - - std::condition_variable doneCondition; - bool done = false; - bool shutdown = false; - - int retVal = EXIT_FAILURE; - - std::thread t([&]() { - { - std::lock_guard lock(mtx); - ready = true; - readyCondition.notify_one(); - } - - nlTestSuite theSuite = { "CHIP DeviceLayer mdns tests", &sTests[0], nullptr, nullptr }; - - nlTestRunner(&theSuite, nullptr); - retVal = nlTestRunnerStats(&theSuite); - - { - std::lock_guard lock(mtx); - done = true; - doneCondition.notify_all(); - } - }); - - { - std::unique_lock lock(mtx); - readyCondition.wait(lock, [&] { return ready; }); - - doneCondition.wait_for(lock, std::chrono::seconds(5)); - if (!done) - { - fprintf(stderr, "mDNS test timeout, is avahi daemon running?\n"); - - // - // This will stop the event loop above, and wait till it has actually stopped - // (i.e exited RunEventLoop()). - // - chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); - chip::Dnssd::ChipDnssdShutdown(); - chip::DeviceLayer::PlatformMgr().Shutdown(); - shutdown = true; - - doneCondition.wait_for(lock, std::chrono::seconds(1)); - if (!done) - { - fprintf(stderr, "Orderly shutdown of the platform main loop failed as well.\n"); - } - retVal = EXIT_FAILURE; - } - } - t.join(); + VerifyOrReturnError(chip::Platform::MemoryInit() == CHIP_NO_ERROR, FAILURE); + VerifyOrReturnError(chip::DeviceLayer::PlatformMgr().InitChipStack() == CHIP_NO_ERROR, FAILURE); + return SUCCESS; +} - if (!shutdown) - { - chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); - chip::Dnssd::ChipDnssdShutdown(); - chip::DeviceLayer::PlatformMgr().Shutdown(); - } +int TestDnssd_Teardown(void * inContext) +{ + chip::DeviceLayer::PlatformMgr().Shutdown(); chip::Platform::MemoryShutdown(); + return SUCCESS; +} + +int TestDnssd() +{ + nlTestSuite theSuite = { "CHIP DeviceLayer mDNS tests", &sTests[0], TestDnssd_Setup, TestDnssd_Teardown }; - return retVal; + // Run test suit against one context. + nlTestRunner(&theSuite, nullptr); + return nlTestRunnerStats(&theSuite); } CHIP_REGISTER_TEST_SUITE(TestDnssd); From 97ac5d9fd29443e537f0eafe1e8ed1ff742328ac Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 25 Apr 2023 16:39:41 +0200 Subject: [PATCH 05/11] Test timeout in case of Avahi not running --- src/platform/tests/TestDnssd.cpp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/platform/tests/TestDnssd.cpp b/src/platform/tests/TestDnssd.cpp index a5b3407e331b62..ed7f374526a3d9 100644 --- a/src/platform/tests/TestDnssd.cpp +++ b/src/platform/tests/TestDnssd.cpp @@ -21,9 +21,7 @@ * */ -#include -#include -#include +#include #include @@ -43,6 +41,8 @@ struct DnssdContext { nlTestSuite * mTestSuite; + std::atomic mTimeoutExpired{ false }; + unsigned int mBrowsedServicesCount = 0; unsigned int mResolvedServicesCount = 0; bool mEndOfInput = false; @@ -102,6 +102,14 @@ static void HandleBrowse(void * context, DnssdService * services, size_t service } } +static void Timeout(chip::System::Layer * systemLayer, void * context) +{ + auto * ctx = static_cast(context); + ChipLogError(DeviceLayer, "mDNS test timeout, is avahi daemon running?"); + ctx->mTimeoutExpired = true; + chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); +} + static void DnssdErrorCallback(void * context, CHIP_ERROR error) { auto * ctx = static_cast(context); @@ -150,11 +158,16 @@ void TestDnssdPubSub(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, chip::Dnssd::ChipDnssdInit(TestDnssdPubSub_DnssdInitCallback, DnssdErrorCallback, &context) == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, + chip::DeviceLayer::SystemLayer().StartTimer(chip::System::Clock::Seconds32(5), Timeout, &context) == + CHIP_NO_ERROR); ChipLogProgress(DeviceLayer, "Start EventLoop"); chip::DeviceLayer::PlatformMgr().RunEventLoop(); ChipLogProgress(DeviceLayer, "End EventLoop"); + NL_TEST_ASSERT(inSuite, !context.mTimeoutExpired); + chip::Dnssd::ChipDnssdShutdown(); } From 1385a7e872297ace3574dc868c5a67da039ea24f Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 25 Apr 2023 16:52:50 +0200 Subject: [PATCH 06/11] Use ChipDnssdStopBrowse before exit --- src/platform/tests/TestDnssd.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/platform/tests/TestDnssd.cpp b/src/platform/tests/TestDnssd.cpp index ed7f374526a3d9..63a6264afed455 100644 --- a/src/platform/tests/TestDnssd.cpp +++ b/src/platform/tests/TestDnssd.cpp @@ -43,6 +43,8 @@ struct DnssdContext std::atomic mTimeoutExpired{ false }; + intptr_t mBrowseIdentifier = 0; + unsigned int mBrowsedServicesCount = 0; unsigned int mResolvedServicesCount = 0; bool mEndOfInput = false; @@ -85,7 +87,9 @@ static void HandleBrowse(void * context, DnssdService * services, size_t service // Make sure that we will not be called again after end-of-input is set NL_TEST_ASSERT(suite, ctx->mEndOfInput == false); - NL_TEST_ASSERT(suite, error == CHIP_NO_ERROR); + // Cancelled error is expected when the browse is stopped with + // ChipDnssdStopBrowse(), so we will not assert on it. + NL_TEST_ASSERT(suite, error == CHIP_NO_ERROR || error == CHIP_ERROR_CANCELLED); ctx->mBrowsedServicesCount += servicesSize; ctx->mEndOfInput = finalBrowse; @@ -145,10 +149,10 @@ static void TestDnssdPubSub_DnssdInitCallback(void * context, CHIP_ERROR error) NL_TEST_ASSERT(suite, ChipDnssdPublishService(&service, HandlePublish, context) == CHIP_NO_ERROR); - intptr_t browseIdentifier; NL_TEST_ASSERT(suite, ChipDnssdBrowse("_mock", DnssdServiceProtocol::kDnssdProtocolTcp, chip::Inet::IPAddressType::kAny, - chip::Inet::InterfaceId::Null(), HandleBrowse, context, &browseIdentifier) == CHIP_NO_ERROR); + chip::Inet::InterfaceId::Null(), HandleBrowse, context, + &ctx->mBrowseIdentifier) == CHIP_NO_ERROR); } void TestDnssdPubSub(nlTestSuite * inSuite, void * inContext) @@ -168,6 +172,9 @@ void TestDnssdPubSub(nlTestSuite * inSuite, void * inContext) NL_TEST_ASSERT(inSuite, !context.mTimeoutExpired); + // Stop browsing so we can safely shutdown DNS-SD + chip::Dnssd::ChipDnssdStopBrowse(context.mBrowseIdentifier); + chip::Dnssd::ChipDnssdShutdown(); } From 51a3a51c9affe7a26d88c3dbaa5352e65691ca89 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Wed, 26 Apr 2023 10:19:21 +0200 Subject: [PATCH 07/11] Fix typo in comment --- src/platform/tests/TestConfigurationMgr.cpp | 2 +- src/platform/tests/TestConnectivityMgr.cpp | 2 +- src/platform/tests/TestDnssd.cpp | 2 +- src/platform/tests/TestKeyValueStoreMgr.cpp | 2 +- src/platform/tests/TestPlatformMgr.cpp | 2 +- src/platform/tests/TestPlatformTime.cpp | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/platform/tests/TestConfigurationMgr.cpp b/src/platform/tests/TestConfigurationMgr.cpp index 91dabe222f28fd..dafe5b209725a2 100644 --- a/src/platform/tests/TestConfigurationMgr.cpp +++ b/src/platform/tests/TestConfigurationMgr.cpp @@ -499,7 +499,7 @@ int TestConfigurationMgr() { nlTestSuite theSuite = { "ConfigurationMgr tests", &sTests[0], TestConfigurationMgr_Setup, TestConfigurationMgr_Teardown }; - // Run test suit against one context. + // Run test suite against one context. nlTestRunner(&theSuite, nullptr); return nlTestRunnerStats(&theSuite); } diff --git a/src/platform/tests/TestConnectivityMgr.cpp b/src/platform/tests/TestConnectivityMgr.cpp index 6cf205737a330a..3a4548c43ef2a7 100644 --- a/src/platform/tests/TestConnectivityMgr.cpp +++ b/src/platform/tests/TestConnectivityMgr.cpp @@ -99,7 +99,7 @@ int TestConnectivityMgr() { nlTestSuite theSuite = { "ConfigurationMgr tests", &sTests[0], TestConnectivityMgr_Setup, TestConnectivityMgr_Teardown }; - // Run test suit against one context. + // Run test suite against one context. nlTestRunner(&theSuite, nullptr); return nlTestRunnerStats(&theSuite); } diff --git a/src/platform/tests/TestDnssd.cpp b/src/platform/tests/TestDnssd.cpp index 63a6264afed455..a72b1b34dbce8d 100644 --- a/src/platform/tests/TestDnssd.cpp +++ b/src/platform/tests/TestDnssd.cpp @@ -198,7 +198,7 @@ int TestDnssd() { nlTestSuite theSuite = { "CHIP DeviceLayer mDNS tests", &sTests[0], TestDnssd_Setup, TestDnssd_Teardown }; - // Run test suit against one context. + // Run test suite against one context. nlTestRunner(&theSuite, nullptr); return nlTestRunnerStats(&theSuite); } diff --git a/src/platform/tests/TestKeyValueStoreMgr.cpp b/src/platform/tests/TestKeyValueStoreMgr.cpp index f189493afe6956..33d70713730461 100644 --- a/src/platform/tests/TestKeyValueStoreMgr.cpp +++ b/src/platform/tests/TestKeyValueStoreMgr.cpp @@ -353,7 +353,7 @@ int TestKeyValueStoreMgr() { nlTestSuite theSuite = { "KeyValueStoreMgr tests", &sTests[0], TestKeyValueStoreMgr_Setup, TestKeyValueStoreMgr_Teardown }; - // Run test suit against one context. + // Run test suite against one context. nlTestRunner(&theSuite, nullptr); return nlTestRunnerStats(&theSuite); } diff --git a/src/platform/tests/TestPlatformMgr.cpp b/src/platform/tests/TestPlatformMgr.cpp index 1e8b5741c70369..9d30b9e636e64a 100644 --- a/src/platform/tests/TestPlatformMgr.cpp +++ b/src/platform/tests/TestPlatformMgr.cpp @@ -247,7 +247,7 @@ int TestPlatformMgr() { nlTestSuite theSuite = { "PlatformMgr tests", &sTests[0], TestPlatformMgr_Setup, TestPlatformMgr_Teardown }; - // Run test suit against one context. + // Run test suite against one context. nlTestRunner(&theSuite, nullptr); return nlTestRunnerStats(&theSuite); } diff --git a/src/platform/tests/TestPlatformTime.cpp b/src/platform/tests/TestPlatformTime.cpp index cca7db7e69c691..8c4e9b838e3dfe 100644 --- a/src/platform/tests/TestPlatformTime.cpp +++ b/src/platform/tests/TestPlatformTime.cpp @@ -125,7 +125,7 @@ int TestPlatformTime() { nlTestSuite theSuite = { "PlatformTime tests", &sTests[0], nullptr, nullptr }; - // Run test suit against one context. + // Run test suite against one context. nlTestRunner(&theSuite, nullptr); return nlTestRunnerStats(&theSuite); } From b637b4d64ddf3cdf5df108b9fcca2ebdfde82277 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Wed, 26 Apr 2023 10:24:17 +0200 Subject: [PATCH 08/11] Cancel timer before stopping event loop --- src/platform/tests/TestDnssd.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/platform/tests/TestDnssd.cpp b/src/platform/tests/TestDnssd.cpp index a72b1b34dbce8d..d0e6a3cc3233b8 100644 --- a/src/platform/tests/TestDnssd.cpp +++ b/src/platform/tests/TestDnssd.cpp @@ -52,6 +52,14 @@ struct DnssdContext } // namespace +static void Timeout(chip::System::Layer * systemLayer, void * context) +{ + auto * ctx = static_cast(context); + ChipLogError(DeviceLayer, "mDNS test timeout, is avahi daemon running?"); + ctx->mTimeoutExpired = true; + chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); +} + static void HandleResolve(void * context, DnssdService * result, const chip::Span & addresses, CHIP_ERROR error) { @@ -74,6 +82,7 @@ static void HandleResolve(void * context, DnssdService * result, const chip::Spa if (ctx->mBrowsedServicesCount == ++ctx->mResolvedServicesCount) { + chip::DeviceLayer::SystemLayer().CancelTimer(Timeout, context); // After last service is resolved, stop the event loop, // so the test case can gracefully exit. chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); @@ -106,14 +115,6 @@ static void HandleBrowse(void * context, DnssdService * services, size_t service } } -static void Timeout(chip::System::Layer * systemLayer, void * context) -{ - auto * ctx = static_cast(context); - ChipLogError(DeviceLayer, "mDNS test timeout, is avahi daemon running?"); - ctx->mTimeoutExpired = true; - chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); -} - static void DnssdErrorCallback(void * context, CHIP_ERROR error) { auto * ctx = static_cast(context); From c60cab3c34ad01f94d1fc8c686a1348d2a1a4bc5 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Wed, 26 Apr 2023 17:12:40 +0200 Subject: [PATCH 09/11] Avoid deadlock when stopping event loop on non-matter thread --- src/platform/tests/TestDnssd.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/platform/tests/TestDnssd.cpp b/src/platform/tests/TestDnssd.cpp index d0e6a3cc3233b8..3006b00f0155e9 100644 --- a/src/platform/tests/TestDnssd.cpp +++ b/src/platform/tests/TestDnssd.cpp @@ -83,9 +83,11 @@ static void HandleResolve(void * context, DnssdService * result, const chip::Spa if (ctx->mBrowsedServicesCount == ++ctx->mResolvedServicesCount) { chip::DeviceLayer::SystemLayer().CancelTimer(Timeout, context); - // After last service is resolved, stop the event loop, - // so the test case can gracefully exit. - chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); + chip::DeviceLayer::SystemLayer().ScheduleLambda([]() { + // After last service is resolved, stop the event loop, + // so the test case can gracefully exit. + chip::DeviceLayer::PlatformMgr().StopEventLoopTask(); + }); } } From f5dbb09fc994eb2c5c3f9b93720f8b5e28876486 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Wed, 26 Apr 2023 21:16:43 +0200 Subject: [PATCH 10/11] Update src/platform/tests/TestDnssd.cpp --- src/platform/tests/TestDnssd.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/platform/tests/TestDnssd.cpp b/src/platform/tests/TestDnssd.cpp index 3006b00f0155e9..4cdb191d65f609 100644 --- a/src/platform/tests/TestDnssd.cpp +++ b/src/platform/tests/TestDnssd.cpp @@ -83,6 +83,11 @@ static void HandleResolve(void * context, DnssdService * result, const chip::Spa if (ctx->mBrowsedServicesCount == ++ctx->mResolvedServicesCount) { chip::DeviceLayer::SystemLayer().CancelTimer(Timeout, context); + // StopEventLoopTask can be called from any thread, but when called from + // non-Matter one it will lock the Matter stack. The same locking rules + // are required when the resolve callback (this one) is called. In order + // to avoid deadlocks, we need to call the StopEventLoopTask from inside + // the Matter event loop by scheduling a lambda. chip::DeviceLayer::SystemLayer().ScheduleLambda([]() { // After last service is resolved, stop the event loop, // so the test case can gracefully exit. From 8d979988acb5d0ddb5aca47d780c785ce151835c Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Fri, 28 Apr 2023 08:04:57 +0200 Subject: [PATCH 11/11] Remove unnecessary comment --- src/platform/tests/TestDnssd.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/platform/tests/TestDnssd.cpp b/src/platform/tests/TestDnssd.cpp index 4cdb191d65f609..d7e4ed18554f74 100644 --- a/src/platform/tests/TestDnssd.cpp +++ b/src/platform/tests/TestDnssd.cpp @@ -15,12 +15,6 @@ * limitations under the License. */ -/** - * @file - * This file implements a unit test suite for the mDNS code functionality. - * - */ - #include #include