[M85] Stop using AutoThread in Thread::Send and make it test only.
Send() was creating an instance of AutoThread for every call,
which is equivalent of instantiatiating a whole new instance of
Thread (AutoThread inherits from Thread) and not just ensuring that
a thread instance is registered for the current thread, as the
comments indicated.
(cherry picked from commit 0fd4c4e630e55644228263dd74f60cb9393656bf)
Bug: webrtc:11908
Change-Id: I8bbb43ca83c30d9f5e1928205b3611271ecad053
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/183441
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#32037}
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/184504
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Guido Urdaneta <guidou@webrtc.org>
Cr-Commit-Position: refs/branch-heads/4183@{#3}
Cr-Branched-From: bf1816170b1f54cf989dea2988496735bf6ddb55-refs/heads/master@{#31567}
diff --git a/rtc_base/thread.cc b/rtc_base/thread.cc
index f8e299a..575c0a6 100644
--- a/rtc_base/thread.cc
+++ b/rtc_base/thread.cc
@@ -32,8 +32,10 @@
#include "rtc_base/atomic_ops.h"
#include "rtc_base/checks.h"
#include "rtc_base/critical_section.h"
+#include "rtc_base/event.h"
#include "rtc_base/logging.h"
#include "rtc_base/null_socket_server.h"
+#include "rtc_base/synchronization/sequence_checker.h"
#include "rtc_base/task_utils/to_queued_task.h"
#include "rtc_base/time_utils.h"
#include "rtc_base/trace_event.h"
@@ -163,6 +165,9 @@
void ThreadManager::RegisterSendAndCheckForCycles(Thread* source,
Thread* target) {
+ RTC_DCHECK(source);
+ RTC_DCHECK(target);
+
CritScope cs(&crit_);
std::deque<Thread*> all_targets({target});
// We check the pre-existing who-sends-to-who graph for any path from target
@@ -889,45 +894,62 @@
AssertBlockingIsAllowedOnCurrentThread();
- AutoThread thread;
Thread* current_thread = Thread::Current();
- RTC_DCHECK(current_thread != nullptr); // AutoThread ensures this
+
#if RTC_DCHECK_IS_ON
- ThreadManager::Instance()->RegisterSendAndCheckForCycles(current_thread,
- this);
-#endif
- bool ready = false;
- PostTask(
- webrtc::ToQueuedTask([msg]() mutable { msg.phandler->OnMessage(&msg); },
- [this, &ready, current_thread] {
- CritScope cs(&crit_);
- ready = true;
- current_thread->socketserver()->WakeUp();
- }));
-
- bool waited = false;
- crit_.Enter();
- while (!ready) {
- crit_.Leave();
- current_thread->socketserver()->Wait(kForever, false);
- waited = true;
- crit_.Enter();
+ if (current_thread) {
+ RTC_DCHECK(current_thread->IsInvokeToThreadAllowed(this));
+ ThreadManager::Instance()->RegisterSendAndCheckForCycles(current_thread,
+ this);
}
- crit_.Leave();
+#endif
- // Our Wait loop above may have consumed some WakeUp events for this
- // Thread, that weren't relevant to this Send. Losing these WakeUps can
- // cause problems for some SocketServers.
- //
- // Concrete example:
- // Win32SocketServer on thread A calls Send on thread B. While processing the
- // message, thread B Posts a message to A. We consume the wakeup for that
- // Post while waiting for the Send to complete, which means that when we exit
- // this loop, we need to issue another WakeUp, or else the Posted message
- // won't be processed in a timely manner.
+ // Perhaps down the line we can get rid of this workaround and always require
+ // current_thread to be valid when Send() is called.
+ std::unique_ptr<rtc::Event> done_event;
+ if (!current_thread)
+ done_event.reset(new rtc::Event());
- if (waited) {
- current_thread->socketserver()->WakeUp();
+ bool ready = false;
+ PostTask(webrtc::ToQueuedTask(
+ [&msg]() mutable { msg.phandler->OnMessage(&msg); },
+ [this, &ready, current_thread, done = done_event.get()] {
+ if (current_thread) {
+ CritScope cs(&crit_);
+ ready = true;
+ current_thread->socketserver()->WakeUp();
+ } else {
+ done->Set();
+ }
+ }));
+
+ if (current_thread) {
+ bool waited = false;
+ crit_.Enter();
+ while (!ready) {
+ crit_.Leave();
+ current_thread->socketserver()->Wait(kForever, false);
+ waited = true;
+ crit_.Enter();
+ }
+ crit_.Leave();
+
+ // Our Wait loop above may have consumed some WakeUp events for this
+ // Thread, that weren't relevant to this Send. Losing these WakeUps can
+ // cause problems for some SocketServers.
+ //
+ // Concrete example:
+ // Win32SocketServer on thread A calls Send on thread B. While processing
+ // the message, thread B Posts a message to A. We consume the wakeup for
+ // that Post while waiting for the Send to complete, which means that when
+ // we exit this loop, we need to issue another WakeUp, or else the Posted
+ // message won't be processed in a timely manner.
+
+ if (waited) {
+ current_thread->socketserver()->WakeUp();
+ }
+ } else {
+ done_event->Wait(rtc::Event::kForever);
}
}
@@ -974,6 +996,50 @@
task.release();
}
+void Thread::AllowInvokesToThread(Thread* thread) {
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
+ if (!IsCurrent()) {
+ PostTask(webrtc::ToQueuedTask(
+ [thread, this]() { AllowInvokesToThread(thread); }));
+ return;
+ }
+ RTC_DCHECK_RUN_ON(this);
+ allowed_threads_.push_back(thread);
+ invoke_policy_enabled_ = true;
+#endif
+}
+
+void Thread::DisallowAllInvokes() {
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
+ if (!IsCurrent()) {
+ PostTask(webrtc::ToQueuedTask([this]() { DisallowAllInvokes(); }));
+ return;
+ }
+ RTC_DCHECK_RUN_ON(this);
+ allowed_threads_.clear();
+ invoke_policy_enabled_ = true;
+#endif
+}
+
+// Returns true if no policies added or if there is at least one policy
+// that permits invocation to |target| thread.
+bool Thread::IsInvokeToThreadAllowed(rtc::Thread* target) {
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
+ RTC_DCHECK_RUN_ON(this);
+ if (!invoke_policy_enabled_) {
+ return true;
+ }
+ for (const auto* thread : allowed_threads_) {
+ if (thread == target) {
+ return true;
+ }
+ }
+ return false;
+#else
+ return true;
+#endif
+}
+
void Thread::PostTask(std::unique_ptr<webrtc::QueuedTask> task) {
// Though Post takes MessageData by raw pointer (last parameter), it still
// takes it with ownership.
diff --git a/rtc_base/thread.h b/rtc_base/thread.h
index e25ed4e..8ae93b4 100644
--- a/rtc_base/thread.h
+++ b/rtc_base/thread.h
@@ -338,6 +338,18 @@
InvokeInternal(posted_from, functor);
}
+ // Allows invoke to specified |thread|. Thread never will be dereferenced and
+ // will be used only for reference-based comparison, so instance can be safely
+ // deleted. If NDEBUG is defined and DCHECK_ALWAYS_ON is undefined do nothing.
+ void AllowInvokesToThread(Thread* thread);
+ // If NDEBUG is defined and DCHECK_ALWAYS_ON is undefined do nothing.
+ void DisallowAllInvokes();
+ // Returns true if |target| was allowed by AllowInvokesToThread() or if no
+ // calls were made to AllowInvokesToThread and DisallowAllInvokes. Otherwise
+ // returns false.
+ // If NDEBUG is defined and DCHECK_ALWAYS_ON is undefined always returns true.
+ bool IsInvokeToThreadAllowed(rtc::Thread* target);
+
// Posts a task to invoke the functor on |this| thread asynchronously, i.e.
// without blocking the thread that invoked PostTask(). Ownership of |functor|
// is passed and (usually, see below) destroyed on |this| thread after it is
@@ -566,6 +578,10 @@
MessageList messages_ RTC_GUARDED_BY(crit_);
PriorityQueue delayed_messages_ RTC_GUARDED_BY(crit_);
uint32_t delayed_next_num_ RTC_GUARDED_BY(crit_);
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
+ std::vector<Thread*> allowed_threads_ RTC_GUARDED_BY(this);
+ bool invoke_policy_enabled_ RTC_GUARDED_BY(this) = false;
+#endif
CriticalSection crit_;
bool fInitialized_;
bool fDestroyed_;
@@ -612,7 +628,9 @@
// AutoThread automatically installs itself at construction
// uninstalls at destruction, if a Thread object is
// _not already_ associated with the current OS thread.
-
+//
+// NOTE: *** This class should only be used by tests ***
+//
class AutoThread : public Thread {
public:
AutoThread();
