pw_thread_freertos: Fix extra wakeups when detaching threads
vTaskResume() is known to cause premature return from blocking APIs,
since it will indiscriminately call prvAddTaskToReadyList().
Instead, use vTaskSuspendAll() / xTaskResumeAll(), which will not wake
up any tasks which weren't already running (and weren't signalled during
scheduler suspension).
This also fixes a latent bug where, in the existing fallback case of
using vTaskSuspendAll(), the new task scheduler state will be
taskSCHEDULER_SUSPENDED, causing the subsequent xTaskResumeAll() to be
skipped.
Bug: b/303885539
Change-Id: I4d09400e133b4d44353d601d07e13917e096a3eb
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/175310
Reviewed-by: Ewout van Bekkum <ewout@google.com>
Reviewed-by: Christoph Klee <chklee@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed-service-accounts.iam.gserviceaccount.com>
Pigweed-Auto-Submit: Austin Foxley <afoxley@google.com>
Reviewed-by: Jonathon Reinhart <jrreinhart@google.com>
diff --git a/pw_thread_freertos/thread.cc b/pw_thread_freertos/thread.cc
index a7e3681..de9f004 100644
--- a/pw_thread_freertos/thread.cc
+++ b/pw_thread_freertos/thread.cc
@@ -193,22 +193,20 @@
void Thread::detach() {
PW_CHECK(joinable());
- if (xTaskGetSchedulerState() == taskSCHEDULER_RUNNING) {
-#if (INCLUDE_vTaskSuspend == 1)
- // No need to suspend extra tasks.
- vTaskSuspend(native_type_->task_handle());
-#else
+ // xTaskResumeAll() can only be used after the scheduler has been started.
+ const bool scheduler_initialized =
+ xTaskGetSchedulerState() != taskSCHEDULER_NOT_STARTED;
+
+ if (scheduler_initialized) {
+ // We don't want to individually suspend and resume this task using
+ // vTaskResume() as that can cause tasks to prematurely wake up and return
+ // from blocking APIs (b/303885539).
vTaskSuspendAll();
-#endif // INCLUDE_vTaskSuspend == 1
}
native_type_->set_detached();
const bool thread_done = native_type_->thread_done();
- if (xTaskGetSchedulerState() == taskSCHEDULER_RUNNING) {
-#if (INCLUDE_vTaskSuspend == 1)
- vTaskResume(native_type_->task_handle());
-#else
+ if (scheduler_initialized) {
xTaskResumeAll();
-#endif // INCLUDE_vTaskSuspend == 1
}
if (thread_done) {
