Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1399)

Unified Diff: webrtc/base/asyncinvoker.cc

Issue 2694723004: Making AsyncInvoker destructor thread-safe. (Closed)
Patch Set: Merge with master Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « webrtc/base/asyncinvoker.h ('k') | webrtc/base/asyncinvoker-inl.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/base/asyncinvoker.cc
diff --git a/webrtc/base/asyncinvoker.cc b/webrtc/base/asyncinvoker.cc
index 77b63a4e66a6f82de6e9ac47c0d1d7d8db00a317..aa65499c960cc268fb658eccae54cbb49083544d 100644
--- a/webrtc/base/asyncinvoker.cc
+++ b/webrtc/base/asyncinvoker.cc
@@ -10,18 +10,29 @@
#include "webrtc/base/asyncinvoker.h"
+#include "webrtc/base/atomicops.h"
#include "webrtc/base/checks.h"
#include "webrtc/base/logging.h"
namespace rtc {
-AsyncInvoker::AsyncInvoker() : destroying_(false) {}
+AsyncInvoker::AsyncInvoker() : invocation_complete_(false, false) {}
AsyncInvoker::~AsyncInvoker() {
destroying_ = true;
SignalInvokerDestroyed();
// Messages for this need to be cleared *before* our destructor is complete.
MessageQueueManager::Clear(this);
+ // And we need to wait for any invocations that are still in progress on
+ // other threads.
+ while (AtomicOps::AcquireLoad(&pending_invocations_)) {
+ // If the destructor was called while AsyncInvoke was being called by
+ // another thread, WITHIN an AsyncInvoked functor, it may do another
+ // Thread::Post even after we called MessageQueueManager::Clear(this). So
+ // we need to keep calling Clear to discard these posts.
+ MessageQueueManager::Clear(this);
+ invocation_complete_.Wait(Event::kForever);
+ }
}
void AsyncInvoker::OnMessage(Message* msg) {
@@ -59,6 +70,7 @@ void AsyncInvoker::DoInvoke(const Location& posted_from,
LOG(LS_WARNING) << "Tried to invoke while destroying the invoker.";
return;
}
+ AtomicOps::Increment(&pending_invocations_);
thread->Post(posted_from, this, id,
new ScopedMessageData<AsyncClosure>(std::move(closure)));
}
@@ -72,6 +84,7 @@ void AsyncInvoker::DoInvokeDelayed(const Location& posted_from,
LOG(LS_WARNING) << "Tried to invoke while destroying the invoker.";
return;
}
+ AtomicOps::Increment(&pending_invocations_);
thread->PostDelayed(posted_from, delay_ms, this, id,
new ScopedMessageData<AsyncClosure>(std::move(closure)));
}
@@ -99,11 +112,16 @@ void GuardedAsyncInvoker::ThreadDestroyed() {
thread_ = nullptr;
}
+AsyncClosure::~AsyncClosure() {
+ AtomicOps::Decrement(&invoker_->pending_invocations_);
+ invoker_->invocation_complete_.Set();
+}
+
NotifyingAsyncClosureBase::NotifyingAsyncClosureBase(
AsyncInvoker* invoker,
const Location& callback_posted_from,
Thread* calling_thread)
- : invoker_(invoker),
+ : AsyncClosure(invoker),
callback_posted_from_(callback_posted_from),
calling_thread_(calling_thread) {
calling_thread->SignalQueueDestroyed.connect(
« no previous file with comments | « webrtc/base/asyncinvoker.h ('k') | webrtc/base/asyncinvoker-inl.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698