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

Unified Diff: webrtc/base/asyncinvoker.cc

Issue 2876273002: Fixing race between ~AsyncInvoker and ~AsyncClosure. (Closed)
Patch Set: Created 3 years, 7 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 cabd8b506d0575ee3e3b41a479f6f7b9fcf63ee8..b4424430393b4dffacfb918a4bfe13bb383c31ac 100644
--- a/webrtc/base/asyncinvoker.cc
+++ b/webrtc/base/asyncinvoker.cc
@@ -16,7 +16,7 @@
namespace rtc {
-AsyncInvoker::AsyncInvoker() : invocation_complete_(false, false) {}
+AsyncInvoker::AsyncInvoker() {}
AsyncInvoker::~AsyncInvoker() {
destroying_ = true;
@@ -25,12 +25,12 @@ AsyncInvoker::~AsyncInvoker() {
// 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.
+ // Release time slice to allow other async-invoked tasks to make progress.
+ rtc::Thread::Current()->SleepMs(0);
nisse-webrtc 2017/05/19 07:16:59 I think it's better with some small but non-zero v
+ // One of the async-invoked tasks in progress could have posted an
+ // additional task to the current thread. So we need to keep calling Clear
+ // to discard these posts.
MessageQueueManager::Clear(this);
nisse-webrtc 2017/05/19 07:16:59 According to the comment, we only need to clear th
- invocation_complete_.Wait(Event::kForever);
}
}
@@ -69,7 +69,6 @@ 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)));
}
@@ -83,7 +82,6 @@ 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)));
}
@@ -111,9 +109,12 @@ void GuardedAsyncInvoker::ThreadDestroyed() {
thread_ = nullptr;
}
+AsyncClosure::AsyncClosure(AsyncInvoker* invoker) : invoker_(invoker) {
+ AtomicOps::Increment(&invoker_->pending_invocations_);
+}
+
AsyncClosure::~AsyncClosure() {
AtomicOps::Decrement(&invoker_->pending_invocations_);
- invoker_->invocation_complete_.Set();
}
NotifyingAsyncClosureBase::NotifyingAsyncClosureBase(
« 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