Ensure task queues delete closures in task queue context.
Bug: webrtc:14449
Change-Id: I90d09d35398c1f8817701662f51cbc6a684a2fe0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275773
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38917}
diff --git a/api/task_queue/BUILD.gn b/api/task_queue/BUILD.gn
index dc69686..69393b8 100644
--- a/api/task_queue/BUILD.gn
+++ b/api/task_queue/BUILD.gn
@@ -65,6 +65,7 @@
":default_task_queue_factory",
":task_queue",
"../../api:field_trials_view",
+ "../../api:make_ref_counted",
"../../api/units:time_delta",
"../../rtc_base:refcount",
"../../rtc_base:rtc_event",
diff --git a/api/task_queue/task_queue_base.h b/api/task_queue/task_queue_base.h
index a2cff9c..f78600d 100644
--- a/api/task_queue/task_queue_base.h
+++ b/api/task_queue/task_queue_base.h
@@ -51,10 +51,16 @@
// Schedules a `task` to execute. Tasks are executed in FIFO order.
// When a TaskQueue is deleted, pending tasks will not be executed but they
- // will be deleted. The deletion of tasks may happen synchronously on the
- // TaskQueue or it may happen asynchronously after TaskQueue is deleted.
- // This may vary from one implementation to the next so assumptions about
- // lifetimes of pending tasks should not be made.
+ // will be deleted.
+ //
+ // As long as tasks are not posted from task destruction, posted tasks are
+ // guaranteed to be destroyed with Current() pointing to the task queue they
+ // were posted to, whether they're executed or not. That means SequenceChecker
+ // works during task destruction, a fact that can be used to guarantee
+ // thread-compatible object deletion happening on a particular task queue
+ // which can simplify class design.
+ // Note that this guarantee does not apply to delayed tasks.
+ //
// May be called on any thread or task queue, including this task queue.
virtual void PostTask(absl::AnyInvocable<void() &&> task) = 0;
diff --git a/api/task_queue/task_queue_test.cc b/api/task_queue/task_queue_test.cc
index 0f6b1d0..7849f42 100644
--- a/api/task_queue/task_queue_test.cc
+++ b/api/task_queue/task_queue_test.cc
@@ -13,7 +13,7 @@
#include "absl/cleanup/cleanup.h"
#include "absl/strings/string_view.h"
-#include "api/task_queue/default_task_queue_factory.h"
+#include "api/task_queue/task_queue_base.h"
#include "api/units/time_delta.h"
#include "rtc_base/event.h"
#include "rtc_base/ref_counter.h"
@@ -22,6 +22,13 @@
namespace webrtc {
namespace {
+// Avoids a dependency to system_wrappers.
+void SleepFor(TimeDelta duration) {
+ rtc::ScopedAllowBaseSyncPrimitivesForTesting allow;
+ rtc::Event event;
+ event.Wait(duration);
+}
+
std::unique_ptr<TaskQueueBase, TaskQueueDeleter> CreateTaskQueue(
const std::unique_ptr<webrtc::TaskQueueFactory>& factory,
absl::string_view task_queue_name,
@@ -147,6 +154,53 @@
EXPECT_FALSE(run.Wait(TimeDelta::Zero())); // and should not run.
}
+TEST_P(TaskQueueTest, PostDelayedHighPrecisionAfterDestruct) {
+ std::unique_ptr<webrtc::TaskQueueFactory> factory = GetParam()(nullptr);
+ rtc::Event run;
+ rtc::Event deleted;
+ auto queue =
+ CreateTaskQueue(factory, "PostDelayedHighPrecisionAfterDestruct");
+ absl::Cleanup cleanup = [&deleted] { deleted.Set(); };
+ queue->PostDelayedHighPrecisionTask(
+ [&run, cleanup = std::move(cleanup)] { run.Set(); },
+ TimeDelta::Millis(100));
+ // Destroy the queue.
+ queue = nullptr;
+ // Task might outlive the TaskQueue, but still should be deleted.
+ EXPECT_TRUE(deleted.Wait(TimeDelta::Seconds(1)));
+ EXPECT_FALSE(run.Wait(TimeDelta::Zero())); // and should not run.
+}
+
+TEST_P(TaskQueueTest, PostedUnexecutedClosureDestroyedOnTaskQueue) {
+ std::unique_ptr<webrtc::TaskQueueFactory> factory = GetParam()(nullptr);
+ auto queue =
+ CreateTaskQueue(factory, "PostedUnexecutedClosureDestroyedOnTaskQueue");
+ TaskQueueBase* queue_ptr = queue.get();
+ queue->PostTask([] { SleepFor(TimeDelta::Millis(100)); });
+ // Give the task queue a chance to start executing the first lambda.
+ SleepFor(TimeDelta::Millis(10));
+ // Then ensure the next lambda (which is likely not executing yet) is
+ // destroyed in the task queue context when the queue is deleted.
+ auto cleanup = absl::Cleanup(
+ [queue_ptr] { EXPECT_EQ(queue_ptr, TaskQueueBase::Current()); });
+ queue->PostTask([cleanup = std::move(cleanup)] {});
+ queue = nullptr;
+}
+
+TEST_P(TaskQueueTest, PostedExecutedClosureDestroyedOnTaskQueue) {
+ std::unique_ptr<webrtc::TaskQueueFactory> factory = GetParam()(nullptr);
+ auto queue =
+ CreateTaskQueue(factory, "PostedExecutedClosureDestroyedOnTaskQueue");
+ TaskQueueBase* queue_ptr = queue.get();
+ // Ensure an executed lambda is destroyed on the task queue.
+ rtc::Event finished;
+ queue->PostTask([cleanup = absl::Cleanup([queue_ptr, &finished] {
+ EXPECT_EQ(queue_ptr, TaskQueueBase::Current());
+ finished.Set();
+ })] {});
+ finished.Wait(rtc::Event::kForever);
+}
+
TEST_P(TaskQueueTest, PostAndReuse) {
std::unique_ptr<webrtc::TaskQueueFactory> factory = GetParam()(nullptr);
rtc::Event event;