From 1445692f6428e907877daf5b541a3d2ae565a721 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Tue, 25 Apr 2023 17:16:32 +0200 Subject: [PATCH] [Linux] Fix event loop task consecutive start/stop (#26195) * [Linux] Fix event loop task consecutive start/stop * Wait for lambdas before stopping loop task * Add comment for sleep() usage in test --- .../GenericPlatformManagerImpl_POSIX.ipp | 4 ++ src/platform/tests/TestPlatformMgr.cpp | 48 +++++++++++++++++-- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.ipp b/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.ipp index 6050950bf4c261..1b09d298eb2867 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.ipp +++ b/src/include/platform/internal/GenericPlatformManagerImpl_POSIX.ipp @@ -59,6 +59,8 @@ CHIP_ERROR GenericPlatformManagerImpl_POSIX::_InitChipStack() // Call up to the base class _InitChipStack() to perform the bulk of the initialization. ReturnErrorOnFailure(GenericPlatformManagerImpl::_InitChipStack()); + mShouldRunEventLoop.store(true, std::memory_order_relaxed); + int ret = pthread_cond_init(&mEventQueueStoppedCond, nullptr); VerifyOrReturnError(ret == 0, CHIP_ERROR_POSIX(ret)); @@ -225,6 +227,8 @@ CHIP_ERROR GenericPlatformManagerImpl_POSIX::_StartEventLoopTask() VerifyOrReturnError(err == 0, CHIP_ERROR_POSIX(err)); #endif + mShouldRunEventLoop.store(true, std::memory_order_relaxed); + // // We need to grab the lock here since we have to protect setting // mHasValidChipTask, which will be read right away upon creating the diff --git a/src/platform/tests/TestPlatformMgr.cpp b/src/platform/tests/TestPlatformMgr.cpp index cd2caab4b96eb9..449e96d30eddbd 100644 --- a/src/platform/tests/TestPlatformMgr.cpp +++ b/src/platform/tests/TestPlatformMgr.cpp @@ -28,6 +28,8 @@ #include #include +#include + #include #include #include @@ -55,14 +57,52 @@ static void TestPlatformMgr_InitShutdown(nlTestSuite * inSuite, void * inContext static void TestPlatformMgr_BasicEventLoopTask(nlTestSuite * inSuite, void * inContext) { + std::atomic counterRun{ 0 }; + CHIP_ERROR err = PlatformMgr().InitChipStack(); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - err = PlatformMgr().StartEventLoopTask(); - NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + // Start/stop the event loop task a few times. + for (size_t i = 0; i < 3; i++) + { + err = PlatformMgr().StartEventLoopTask(); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + std::atomic counterSync{ 2 }; + + // Verify that the event loop will not exit until we tell it to by + // scheduling few lambdas (for the test to pass, the event loop will + // have to process more than one event). + DeviceLayer::SystemLayer().ScheduleLambda([&]() { + counterRun++; + counterSync--; + }); + + // Sleep for a short time to allow the event loop to process the + // scheduled event and go to idle state. Without this sleep, the + // event loop may process both scheduled lambdas during single + // iteration of the event loop which would defeat the purpose of + // this test on POSIX platforms where the event loop is implemented + // using a "do { ... } while (shouldRun)" construct. + chip::test_utils::SleepMillis(10); + + DeviceLayer::SystemLayer().ScheduleLambda([&]() { + counterRun++; + counterSync--; + }); + + // Wait for the event loop to process the scheduled events. + // Note, that we can not use any synchronization primitives like + // condition variables or barriers, because the test has to compile + // on all platforms. Instead we use a busy loop with a timeout. + for (size_t t = 0; counterSync != 0 && t < 1000; t++) + chip::test_utils::SleepMillis(1); + + err = PlatformMgr().StopEventLoopTask(); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + } - err = PlatformMgr().StopEventLoopTask(); - NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, counterRun == (3 * 2)); PlatformMgr().Shutdown(); }