Reland "Refactor the PlatformThread API."

This reverts commit 793bac569fdf1be16cbf24d7871d20d00bbec81b.

Reason for revert: rare compilation error fixed

Original change's description:
> Revert "Refactor the PlatformThread API."
>
> This reverts commit c89fdd716c4c8af608017c76f75bf27e4c3d602e.
>
> Reason for revert: Causes rare compilation error on win-libfuzzer-asan trybot.
> See https://ci.chromium.org/p/chromium/builders/try/win-libfuzzer-asan-rel/713745?
>
> Original change's description:
> > Refactor the PlatformThread API.
> >
> > PlatformThread's API is using old style function pointers, causes
> > casting, is unintuitive and forces artificial call sequences, and
> > is additionally possible to misuse in release mode.
> >
> > Fix this by an API face lift:
> > 1. The class is turned into a handle, which can be empty.
> > 2. The only way of getting a non-empty PlatformThread is by calling
> > SpawnJoinable or SpawnDetached, clearly conveying the semantics to the
> > code reader.
> > 3. Handles can be Finalized, which works differently for joinable and
> > detached threads:
> >   a) Handles for detached threads are simply closed where applicable.
> >   b) Joinable threads are joined before handles are closed.
> > 4. The destructor finalizes handles. No explicit call is needed.
> >
> > Fixed: webrtc:12727
> > Change-Id: Id00a0464edf4fc9e552b6a1fbb5d2e1280e88811
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215075
> > Commit-Queue: Markus Handell <handellm@webrtc.org>
> > Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> > Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> > Reviewed-by: Tommi <tommi@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#33923}
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> TBR=handellm@webrtc.org
>
> Bug: webrtc:12727
> Change-Id: Ic0146be8866f6dd3ad9c364fb8646650b8e07419
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217583
> Reviewed-by: Guido Urdaneta <guidou@webrtc.org>
> Reviewed-by: Markus Handell <handellm@webrtc.org>
> Commit-Queue: Guido Urdaneta <guidou@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#33936}

# Not skipping CQ checks because this is a reland.

Bug: webrtc:12727
Change-Id: Ifd6f44eac72fed84474277a1be03eb84d2f4376e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/217881
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Markus Handell <handellm@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33950}
This commit is contained in:
Markus Handell
2021-05-07 15:02:36 +02:00
committed by WebRTC LUCI CQ
parent f95536dd5a
commit ad5037b4a8
38 changed files with 588 additions and 865 deletions

View File

@ -48,7 +48,6 @@ ProcessThreadImpl::ProcessThreadImpl(const char* thread_name)
ProcessThreadImpl::~ProcessThreadImpl() {
RTC_DCHECK(thread_checker_.IsCurrent());
RTC_DCHECK(!thread_.get());
RTC_DCHECK(!stop_);
while (!delayed_tasks_.empty()) {
@ -72,8 +71,8 @@ void ProcessThreadImpl::Delete() {
// Doesn't need locking, because the contending thread isn't running.
void ProcessThreadImpl::Start() RTC_NO_THREAD_SAFETY_ANALYSIS {
RTC_DCHECK(thread_checker_.IsCurrent());
RTC_DCHECK(!thread_.get());
if (thread_.get())
RTC_DCHECK(thread_.empty());
if (!thread_.empty())
return;
RTC_DCHECK(!stop_);
@ -81,14 +80,18 @@ void ProcessThreadImpl::Start() RTC_NO_THREAD_SAFETY_ANALYSIS {
for (ModuleCallback& m : modules_)
m.module->ProcessThreadAttached(this);
thread_.reset(
new rtc::PlatformThread(&ProcessThreadImpl::Run, this, thread_name_));
thread_->Start();
thread_ = rtc::PlatformThread::SpawnJoinable(
[this] {
CurrentTaskQueueSetter set_current(this);
while (Process()) {
}
},
thread_name_);
}
void ProcessThreadImpl::Stop() {
RTC_DCHECK(thread_checker_.IsCurrent());
if (!thread_.get())
if (thread_.empty())
return;
{
@ -98,9 +101,7 @@ void ProcessThreadImpl::Stop() {
}
wake_up_.Set();
thread_->Stop();
thread_.reset();
thread_.Finalize();
StopNoLocks();
}
@ -108,7 +109,7 @@ void ProcessThreadImpl::Stop() {
// No locking needed, since this is called after the contending thread is
// stopped.
void ProcessThreadImpl::StopNoLocks() RTC_NO_THREAD_SAFETY_ANALYSIS {
RTC_DCHECK(!thread_);
RTC_DCHECK(thread_.empty());
stop_ = false;
for (ModuleCallback& m : modules_)
@ -199,7 +200,7 @@ void ProcessThreadImpl::RegisterModule(Module* module,
// Now that we know the module isn't in the list, we'll call out to notify
// the module that it's attached to the worker thread. We don't hold
// the lock while we make this call.
if (thread_.get())
if (!thread_.empty())
module->ProcessThreadAttached(this);
{
@ -227,14 +228,6 @@ void ProcessThreadImpl::DeRegisterModule(Module* module) {
module->ProcessThreadAttached(nullptr);
}
// static
void ProcessThreadImpl::Run(void* obj) {
ProcessThreadImpl* impl = static_cast<ProcessThreadImpl*>(obj);
CurrentTaskQueueSetter set_current(impl);
while (impl->Process()) {
}
}
bool ProcessThreadImpl::Process() {
TRACE_EVENT1("webrtc", "ProcessThreadImpl", "name", thread_name_);
int64_t now = rtc::TimeMillis();