Fixing race between ~AsyncInvoker and ~AsyncClosure, using ref-counting.
The AsyncInvoker destructor waits for all invoked tasks to be complete
(in other words, all AsyncClosures to be destructed). They were using an
event to wake up the destructor, but a race made it possible for this
event to be dereferenced after it's destroyed.
This CL makes the event reference counted, such that if the destructor
runs right after AsyncClosure decrements "pending_invocations_",
setting the event will be a no-op, and the event will be destructed
in the AsyncClosure destructor.
This CL also fixes a deadlock that may occur for "re-entrant"
invocations. The deadlock occurs if the AsyncInvoker is destroyed on
thread A while a task on thread B is running, which AsyncInvokes a task
back on thread A.
This was causing pending_invocations_ to end up negative, because
an AsyncClosure that's never added to a thread's message queue (due to
the "destroying_" flag) caused the count to be decremented but not
incremented.
BUG=webrtc:7656
Review-Url: https://codereview.webrtc.org/2885143005
Cr-Commit-Position: refs/heads/master@{#19278}
diff --git a/webrtc/rtc_base/asyncinvoker-inl.h b/webrtc/rtc_base/asyncinvoker-inl.h
index 5b2cf4e..7878b15 100644
--- a/webrtc/rtc_base/asyncinvoker-inl.h
+++ b/webrtc/rtc_base/asyncinvoker-inl.h
@@ -11,10 +11,12 @@
#ifndef WEBRTC_RTC_BASE_ASYNCINVOKER_INL_H_
#define WEBRTC_RTC_BASE_ASYNCINVOKER_INL_H_
-#include "webrtc/rtc_base/atomicops.h"
#include "webrtc/rtc_base/bind.h"
#include "webrtc/rtc_base/criticalsection.h"
+#include "webrtc/rtc_base/event.h"
#include "webrtc/rtc_base/messagehandler.h"
+#include "webrtc/rtc_base/refcountedobject.h"
+#include "webrtc/rtc_base/scoped_ref_ptr.h"
#include "webrtc/rtc_base/sigslot.h"
#include "webrtc/rtc_base/thread.h"
#include "webrtc/rtc_base/thread_annotations.h"
@@ -27,7 +29,7 @@
// on the calling thread if necessary.
class AsyncClosure {
public:
- explicit AsyncClosure(AsyncInvoker* invoker) : invoker_(invoker) {}
+ explicit AsyncClosure(AsyncInvoker* invoker);
virtual ~AsyncClosure();
// Runs the asynchronous task, and triggers a callback to the calling
// thread if needed. Should be called from the target thread.
@@ -35,6 +37,11 @@
protected:
AsyncInvoker* invoker_;
+ // Reference counted so that if the AsyncInvoker destructor finishes before
+ // an AsyncClosure's destructor that's about to call
+ // "invocation_complete_->Set()", it's not dereferenced after being
+ // destroyed.
+ scoped_refptr<RefCountedObject<Event>> invocation_complete_;
};
// Simple closure that doesn't trigger a callback for the calling thread.