From bf793325305880dc20d6820351bb104edb26e864 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Tue, 5 May 2026 12:12:37 -0700 Subject: [PATCH] [tasklet] fix `Unpost()` behavior during tasklet processing (#13039) This commit fixes an issue where a tasklet could not be successfully unposted if it was already scheduled for execution in the current event loop iteration. Previously, `Scheduler::ProcessQueuedTasklets()` copied and cleared the queued tasklets before running them. If a running tasklet called `Unpost()` on another tasklet that was also in the copied list, the unpost operation would fail to remove it because it only checked the main queue. To address this, the `Scheduler` now explicitly maintains two separate queues: `mPostedQueue` and `mRuningQueue`. The `Tasklet::Unpost()` method is updated to remove the target tasklet from both queues, ensuring it is correctly dequeued even if it is pending in the running list. The queue logic is encapsulated into a nested `Queue` class to manage the circular singly linked-list operations cleanly. Additionally, unit tests are expanded to cover scenarios where tasklets post or unpost other tasklets during execution. --- src/core/common/tasklet.cpp | 73 ++++++----- src/core/common/tasklet.hpp | 32 +++-- tests/unit/test_tasklet.cpp | 237 ++++++++++++++++++++++++++++++++++++ 3 files changed, 302 insertions(+), 40 deletions(-) diff --git a/src/core/common/tasklet.cpp b/src/core/common/tasklet.cpp index d45fa9ac2..304f172b0 100644 --- a/src/core/common/tasklet.cpp +++ b/src/core/common/tasklet.cpp @@ -42,19 +42,17 @@ void Tasklet::Post(void) { if (!IsPosted()) { - Get().PostTasklet(*this); + Get().mPostedQueue.PostTasklet(*this); } } void Tasklet::Unpost(void) { - if (IsPosted()) - { - Get().RemoveTasklet(*this); - } + Get().mPostedQueue.RemoveTasklet(*this); + Get().mRunningQueue.RemoveTasklet(*this); } -void Tasklet::Scheduler::PostTasklet(Tasklet &aTasklet) +void Tasklet::Scheduler::Queue::PostTasklet(Tasklet &aTasklet) { // Tasklets are saved in a circular singly linked list. @@ -72,13 +70,24 @@ void Tasklet::Scheduler::PostTasklet(Tasklet &aTasklet) } } -void Tasklet::Scheduler::RemoveTasklet(Tasklet &aTasklet) +void Tasklet::Scheduler::Queue::RemoveTasklet(Tasklet &aTasklet) { - Tasklet *prev = mTail; + Tasklet *prev; + + VerifyOrExit(aTasklet.IsPosted()); + + VerifyOrExit(!IsEmpty()); + + prev = mTail; while (prev->mNext != &aTasklet) { prev = prev->mNext; + + if (prev == mTail) + { + ExitNow(); + } } prev->mNext = aTasklet.mNext; @@ -88,34 +97,42 @@ void Tasklet::Scheduler::RemoveTasklet(Tasklet &aTasklet) { mTail = (prev != &aTasklet) ? prev : nullptr; } + +exit: + return; +} + +Tasklet *Tasklet::Scheduler::Queue::PopTasklet(void) +{ + Tasklet *tasklet; + + if (IsEmpty()) + { + tasklet = nullptr; + } + else + { + tasklet = mTail->mNext; + RemoveTasklet(*tasklet); + } + + return tasklet; } void Tasklet::Scheduler::ProcessQueuedTasklets(void) { - Tasklet *tail = mTail; + Tasklet *tasklet; - // This method processes all tasklets queued when this is called. We - // keep a copy the current list and then clear the main list by - // setting `mTail` to `nullptr`. A newly posted tasklet while - // processing the currently queued tasklets will then trigger a call - // to `otTaskletsSignalPending()`. + // We transfer all currently posted tasklets to the `mRunningQueue` and + // clear the `mPostedQueue`. This ensures that any new tasklet posted + // while we are processing `mRunningQueue` will be added to `mPostedQueue` + // and will trigger a call to `otTaskletsSignalPending()`. - mTail = nullptr; + mRunningQueue = mPostedQueue; + mPostedQueue.Clear(); - while (tail != nullptr) + while ((tasklet = mRunningQueue.PopTasklet()) != nullptr) { - Tasklet *tasklet = tail->mNext; - - if (tasklet == tail) - { - tail = nullptr; - } - else - { - tail->mNext = tasklet->mNext; - } - - tasklet->mNext = nullptr; tasklet->RunTask(); } } diff --git a/src/core/common/tasklet.hpp b/src/core/common/tasklet.hpp index 5370fe8d3..5d3ee465e 100644 --- a/src/core/common/tasklet.hpp +++ b/src/core/common/tasklet.hpp @@ -68,21 +68,13 @@ public: friend class Tasklet; public: - /** - * Initializes the object. - */ - Scheduler(void) - : mTail(nullptr) - { - } - /** * Indicates whether or not there are tasklets pending. * * @retval TRUE If there are tasklets pending. * @retval FALSE If there are no tasklets pending. */ - bool AreTaskletsPending(void) const { return mTail != nullptr; } + bool AreTaskletsPending(void) const { return !mPostedQueue.IsEmpty(); } /** * Processes all tasklets queued when this is called. @@ -90,10 +82,26 @@ public: void ProcessQueuedTasklets(void); private: - void PostTasklet(Tasklet &aTasklet); - void RemoveTasklet(Tasklet &aTasklet); + class Queue // A circular singly linked-list + { + public: + Queue(void) + : mTail(nullptr) + { + } - Tasklet *mTail; // A circular singly linked-list + void Clear(void) { mTail = nullptr; } + bool IsEmpty(void) const { return (mTail == nullptr); } + void PostTasklet(Tasklet &aTasklet); + void RemoveTasklet(Tasklet &aTasklet); + Tasklet *PopTasklet(void); + + private: + Tasklet *mTail; + }; + + Queue mPostedQueue; + Queue mRunningQueue; }; /** diff --git a/tests/unit/test_tasklet.cpp b/tests/unit/test_tasklet.cpp index 06c1b71cd..b593ca4fb 100644 --- a/tests/unit/test_tasklet.cpp +++ b/tests/unit/test_tasklet.cpp @@ -38,9 +38,12 @@ namespace ot { #define Log(aMessage) fprintf(stderr, aMessage "\n\r") static Instance *sInstance = nullptr; +static Tasklet *sTask1 = nullptr; static bool sTask1Handled = false; static bool sTask2Handled = false; static bool sTask3Handled = false; +static bool sTask4Handled = false; +static bool sTask5Handled = false; static bool sSignalPendingCalled = false; static bool sShouldTask3RepostItself = false; @@ -61,6 +64,8 @@ void ResetTestFlags(void) sTask1Handled = false; sTask2Handled = false; sTask3Handled = false; + sTask4Handled = false; + sTask5Handled = false; sSignalPendingCalled = false; } @@ -99,6 +104,26 @@ void HandleTask3(Tasklet &aTasklet) } } +void HandleTask4(Tasklet &aTasklet) +{ + Log(" HandleTask4() - will post task1"); + CheckTaskeltFromHandler(aTasklet); + VerifyOrQuit(!sTask4Handled); + sTask4Handled = true; + + sTask1->Post(); +} + +void HandleTask5(Tasklet &aTasklet) +{ + Log(" HandleTask5() - will un-post task1"); + CheckTaskeltFromHandler(aTasklet); + VerifyOrQuit(!sTask5Handled); + sTask5Handled = true; + + sTask1->Unpost(); +} + void TestTasklet(void) { Log("TestTasklet"); @@ -111,6 +136,10 @@ void TestTasklet(void) Tasklet task1(*sInstance, HandleTask1); Tasklet task2(*sInstance, HandleTask2); Tasklet task3(*sInstance, HandleTask3); + Tasklet task4(*sInstance, HandleTask4); + Tasklet task5(*sInstance, HandleTask5); + + sTask1 = &task1; Log("Process all initially posted tasks after `Instance` initialization"); @@ -563,6 +592,214 @@ void TestTasklet(void) VerifyOrQuit(!sSignalPendingCalled); VerifyOrQuit(!scheduler.AreTaskletsPending()); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + Log("Post task4 (which posts task1 from its handler) and then task1"); + Log("We expect the posting of task1 (from task4 handler) to be ignored since it is already posted"); + + ResetTestFlags(); + + task4.Post(); + task1.Post(); + + VerifyOrQuit(task4.IsPosted()); + VerifyOrQuit(task1.IsPosted()); + + VerifyOrQuit(sSignalPendingCalled); + sSignalPendingCalled = false; + + scheduler.ProcessQueuedTasklets(); + + VerifyOrQuit(sTask4Handled); + VerifyOrQuit(sTask1Handled); + + VerifyOrQuit(!task4.IsPosted()); + VerifyOrQuit(!task1.IsPosted()); + + VerifyOrQuit(!sSignalPendingCalled); + VerifyOrQuit(!scheduler.AreTaskletsPending()); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + Log("Post task1 first and then task4 (which posts task1 from its handler) now"); + Log("Since task1 is first, it should be handled before task4's handler and task4's handler should post task1"); + + ResetTestFlags(); + + task1.Post(); + task4.Post(); + + VerifyOrQuit(task4.IsPosted()); + VerifyOrQuit(task1.IsPosted()); + + VerifyOrQuit(sSignalPendingCalled); + sSignalPendingCalled = false; + + scheduler.ProcessQueuedTasklets(); + + VerifyOrQuit(sTask4Handled); + VerifyOrQuit(sTask1Handled); + + VerifyOrQuit(!task4.IsPosted()); + VerifyOrQuit(task1.IsPosted()); + + VerifyOrQuit(sSignalPendingCalled); + VerifyOrQuit(scheduler.AreTaskletsPending()); + + ResetTestFlags(); + + scheduler.ProcessQueuedTasklets(); + VerifyOrQuit(!sTask4Handled); + VerifyOrQuit(sTask1Handled); + + VerifyOrQuit(!task4.IsPosted()); + VerifyOrQuit(!task1.IsPosted()); + + VerifyOrQuit(!sSignalPendingCalled); + VerifyOrQuit(!scheduler.AreTaskletsPending()); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + Log("Post task5 (which un-posts task1 from its handler) and then task1"); + Log("Since task5 is first, we expect task1 to be removed and its handler never called"); + + ResetTestFlags(); + + task5.Post(); + task1.Post(); + + VerifyOrQuit(task5.IsPosted()); + VerifyOrQuit(task1.IsPosted()); + + VerifyOrQuit(sSignalPendingCalled); + sSignalPendingCalled = false; + + scheduler.ProcessQueuedTasklets(); + + VerifyOrQuit(sTask5Handled); + VerifyOrQuit(!sTask1Handled); + + VerifyOrQuit(!task5.IsPosted()); + VerifyOrQuit(!task1.IsPosted()); + + VerifyOrQuit(!sSignalPendingCalled); + VerifyOrQuit(!scheduler.AreTaskletsPending()); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + Log("Post task1 first, then task5 (which un-posts task1 from its handler)"); + Log("Both tasks should be handled"); + + ResetTestFlags(); + + task1.Post(); + task5.Post(); + + VerifyOrQuit(task1.IsPosted()); + VerifyOrQuit(task5.IsPosted()); + + VerifyOrQuit(sSignalPendingCalled); + sSignalPendingCalled = false; + + scheduler.ProcessQueuedTasklets(); + + VerifyOrQuit(sTask1Handled); + VerifyOrQuit(sTask5Handled); + + VerifyOrQuit(!task5.IsPosted()); + VerifyOrQuit(!task1.IsPosted()); + + VerifyOrQuit(!sSignalPendingCalled); + VerifyOrQuit(!scheduler.AreTaskletsPending()); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + Log("Post task5 on its own, which un-posts task1 from its handler - it should do nothing to task1"); + + ResetTestFlags(); + + task5.Post(); + + VerifyOrQuit(task5.IsPosted()); + + VerifyOrQuit(sSignalPendingCalled); + sSignalPendingCalled = false; + + scheduler.ProcessQueuedTasklets(); + + VerifyOrQuit(sTask5Handled); + + VerifyOrQuit(!task5.IsPosted()); + VerifyOrQuit(!task1.IsPosted()); + + VerifyOrQuit(!sSignalPendingCalled); + VerifyOrQuit(!scheduler.AreTaskletsPending()); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + Log("Post task4 (which posts task1 from its handler), then task5 (which un-posts task1 from its handler)"); + Log("The two should cancel each other and at the end task1 should not be posted"); + + ResetTestFlags(); + + task4.Post(); + task5.Post(); + + VerifyOrQuit(task4.IsPosted()); + VerifyOrQuit(task5.IsPosted()); + VerifyOrQuit(!task1.IsPosted()); + + VerifyOrQuit(sSignalPendingCalled); + sSignalPendingCalled = false; + + scheduler.ProcessQueuedTasklets(); + + VerifyOrQuit(sTask4Handled); + VerifyOrQuit(sTask5Handled); + VerifyOrQuit(!sTask1Handled); + + VerifyOrQuit(!task5.IsPosted()); + VerifyOrQuit(!task4.IsPosted()); + VerifyOrQuit(!task1.IsPosted()); + + VerifyOrQuit(sSignalPendingCalled); + VerifyOrQuit(!scheduler.AreTaskletsPending()); + + //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + Log("Post task5 (which un-posts task1 from its handler), then task1, and then task4 (which posts task1)"); + Log("The task1 should be removed while processing task5, but then posted again from task4"); + + ResetTestFlags(); + + task5.Post(); + task1.Post(); + task4.Post(); + + VerifyOrQuit(task4.IsPosted()); + VerifyOrQuit(task5.IsPosted()); + VerifyOrQuit(task1.IsPosted()); + + VerifyOrQuit(sSignalPendingCalled); + sSignalPendingCalled = false; + + scheduler.ProcessQueuedTasklets(); + + VerifyOrQuit(sTask5Handled); + VerifyOrQuit(sTask4Handled); + VerifyOrQuit(!sTask1Handled); + + VerifyOrQuit(!task5.IsPosted()); + VerifyOrQuit(!task4.IsPosted()); + VerifyOrQuit(task1.IsPosted()); + + VerifyOrQuit(sSignalPendingCalled); + VerifyOrQuit(scheduler.AreTaskletsPending()); + + // Handle the posted task1 + ResetTestFlags(); + + scheduler.ProcessQueuedTasklets(); + VerifyOrQuit(sTask1Handled); + + VerifyOrQuit(!task1.IsPosted()); + + VerifyOrQuit(!sSignalPendingCalled); + VerifyOrQuit(!scheduler.AreTaskletsPending()); } }