Proposed ~TaskQueue() solution - GCD

webrtc/rtc_base/task_queue_gcd.cc

 /*
  *  Copyright 2016 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 35 matching lines @@
   static void SetNotActive(void* context) {
     QueueContext* qc = static_cast<QueueContext*>(context);
     qc->is_active = false;
   }
 
   static void DeleteContext(void* context) {
     QueueContext* qc = static_cast<QueueContext*>(context);
     delete qc;
   }
 
+  // By posting this function synchronously, one can wait until the queue
+  // has been emptied.
+  static void VoidFunction(void* context) {}
+
   TaskQueue* const queue;
   bool is_active;
 };
 
 struct TaskQueue::TaskContext {
   TaskContext(QueueContext* queue_ctx, std::unique_ptr<QueuedTask> task)
       : queue_ctx(queue_ctx), task(std::move(task)) {}
   virtual ~TaskContext() {}
 
   static void RunTask(void* context) {
@@ skipping 61 matching lines @@
 
 TaskQueue::~TaskQueue() {
   RTC_DCHECK(!IsCurrent());
   // Implementation/behavioral note:
   // Dispatch queues are reference counted via calls to dispatch_retain and
   // dispatch_release. Pending blocks submitted to a queue also hold a
   // reference to the queue until they have finished. Once all references to a
   // queue have been released, the queue will be deallocated by the system.
   // This is why we check the context before running tasks.
 
-  // Use dispatch_sync to set the context to null to guarantee that there's not
-  // a race between checking the context and using it from a task.
-  dispatch_sync_f(queue_, context_, &QueueContext::SetNotActive);
+  // Toggle the context to not-active while the task-queue is suspended, to
+  // avoid a race.
+  dispatch_suspend(queue_);

[nisse-webrtc, 2017/08/30 09:20:35]

The interesting question is what dispatch_suspend does, if called concurrently
with a running task. If it blocks until the current task is finished, there
should be no race here.

[eladalon, 2017/08/30 09:41:27]

My thinking was that it would be helpful in either of these two possibilities:

* dispatch_suspend() blocks until the current task completes, and the next
pending task will not start executing until resume() is called.
* dispatch_suspend() actually pauses the currently running task.

However, some initial probing makes me suspect neither of these interpretations
is correct. Let me dig some more; I'll post an update later today.

[eladalon, 2017/08/30 10:21:42]

Okay, looks like it prevents the next task from launching, but given that the
current task might already be before/after the point where it checks the flag, I
don't see how suspend/resume helps in providing a better guarantee. We might as
well just call SetNotActive() directly.

+  QueueContext::SetNotActive(context_);
+  dispatch_resume(queue_);
+
+  // Wait until all previous tasks' destructors have been invoked.
+  dispatch_sync_f(queue_, context_, &QueueContext::VoidFunction);
+

[eladalon, 2017/08/30 09:19:36]

Might not hurt to add an RTC_DCHECK here to make sure nobody has posted any
additional tasks to the queue while we were waiting for VoidFunction() to be
executed; posting tasks to a TaskQueue while it's being torn down is an error.

   dispatch_release(queue_);
 }
 
 // static
 TaskQueue* TaskQueue::Current() {
   return static_cast<TaskQueue*>(pthread_getspecific(GetQueuePtrTls()));
 }
 
 // static
 bool TaskQueue::IsCurrent(const char* queue_name) {
@@ skipping 27 matching lines @@
       context_, std::move(task), reply_queue->context_, std::move(reply));
   dispatch_async_f(queue_, context, &PostTaskAndReplyContext::RunTask);
 }
 
 void TaskQueue::PostTaskAndReply(std::unique_ptr<QueuedTask> task,
                                  std::unique_ptr<QueuedTask> reply) {
   return PostTaskAndReply(std::move(task), std::move(reply), Current());
 }
 
 }  // namespace rtc