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.

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-Commit-Position: refs/heads/master@{#32037}
This commit is contained in:
Tomas Gunnarsson 2020-09-04 16:33:25 +02:00 committed by Commit Bot
parent d46db9f152
commit 0fd4c4e630
2 changed files with 58 additions and 36 deletions

View file

@ -32,6 +32,7 @@
#include "rtc_base/atomic_ops.h"
#include "rtc_base/checks.h"
#include "rtc_base/deprecated/recursive_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"
@ -164,6 +165,9 @@ void ThreadManager::RemoveFromSendGraph(Thread* thread) {
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
@ -890,46 +894,62 @@ void Thread::Send(const Location& posted_from,
AssertBlockingIsAllowedOnCurrentThread();
AutoThread thread;
Thread* current_thread = Thread::Current();
RTC_DCHECK(current_thread != nullptr); // AutoThread ensures this
RTC_DCHECK(current_thread->IsInvokeToThreadAllowed(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);
}
}

View file

@ -625,7 +625,9 @@ class RTC_LOCKABLE RTC_EXPORT Thread : public webrtc::TaskQueueBase {
// 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();