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}
diff --git a/rtc_base/task_queue_win.cc b/rtc_base/task_queue_win.cc
index 8bfe5e5..d797d47 100644
--- a/rtc_base/task_queue_win.cc
+++ b/rtc_base/task_queue_win.cc
@@ -29,16 +29,18 @@
#include <utility>
#include "absl/strings/string_view.h"
+#include "absl/types/optional.h"
#include "api/task_queue/queued_task.h"
#include "api/task_queue/task_queue_base.h"
#include "rtc_base/arraysize.h"
#include "rtc_base/checks.h"
+#include "rtc_base/constructor_magic.h"
#include "rtc_base/event.h"
#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
#include "rtc_base/platform_thread.h"
-#include "rtc_base/time_utils.h"
#include "rtc_base/synchronization/mutex.h"
+#include "rtc_base/time_utils.h"
namespace webrtc {
namespace {
@@ -56,16 +58,12 @@
TaskQueueFactory::Priority priority) {
switch (priority) {
case TaskQueueFactory::Priority::HIGH:
- return rtc::kRealtimePriority;
+ return rtc::ThreadPriority::kRealtime;
case TaskQueueFactory::Priority::LOW:
- return rtc::kLowPriority;
+ return rtc::ThreadPriority::kLow;
case TaskQueueFactory::Priority::NORMAL:
- return rtc::kNormalPriority;
- default:
- RTC_NOTREACHED();
- break;
+ return rtc::ThreadPriority::kNormal;
}
- return rtc::kNormalPriority;
}
int64_t GetTick() {
@@ -167,24 +165,6 @@
void RunPendingTasks();
private:
- static void ThreadMain(void* context);
-
- class WorkerThread : public rtc::PlatformThread {
- public:
- WorkerThread(rtc::ThreadRunFunction func,
- void* obj,
- absl::string_view thread_name,
- rtc::ThreadPriority priority)
- : PlatformThread(func,
- obj,
- thread_name,
- rtc::ThreadAttributes().SetPriority(priority)) {}
-
- bool QueueAPC(PAPCFUNC apc_function, ULONG_PTR data) {
- return rtc::PlatformThread::QueueAPC(apc_function, data);
- }
- };
-
void RunThreadMain();
bool ProcessQueuedMessages();
void RunDueTasks();
@@ -207,7 +187,7 @@
greater<DelayedTaskInfo>>
timer_tasks_;
UINT_PTR timer_id_ = 0;
- WorkerThread thread_;
+ rtc::PlatformThread thread_;
Mutex pending_lock_;
std::queue<std::unique_ptr<QueuedTask>> pending_
RTC_GUARDED_BY(pending_lock_);
@@ -216,10 +196,12 @@
TaskQueueWin::TaskQueueWin(absl::string_view queue_name,
rtc::ThreadPriority priority)
- : thread_(&TaskQueueWin::ThreadMain, this, queue_name, priority),
- in_queue_(::CreateEvent(nullptr, true, false, nullptr)) {
+ : in_queue_(::CreateEvent(nullptr, true, false, nullptr)) {
RTC_DCHECK(in_queue_);
- thread_.Start();
+ thread_ = rtc::PlatformThread::SpawnJoinable(
+ [this] { RunThreadMain(); }, queue_name,
+ rtc::ThreadAttributes().SetPriority(priority));
+
rtc::Event event(false, false);
RTC_CHECK(thread_.QueueAPC(&InitializeQueueThread,
reinterpret_cast<ULONG_PTR>(&event)));
@@ -228,11 +210,13 @@
void TaskQueueWin::Delete() {
RTC_DCHECK(!IsCurrent());
- while (!::PostThreadMessage(thread_.GetThreadRef(), WM_QUIT, 0, 0)) {
+ RTC_CHECK(thread_.GetHandle() != absl::nullopt);
+ while (
+ !::PostThreadMessage(GetThreadId(*thread_.GetHandle()), WM_QUIT, 0, 0)) {
RTC_CHECK_EQ(ERROR_NOT_ENOUGH_QUOTA, ::GetLastError());
Sleep(1);
}
- thread_.Stop();
+ thread_.Finalize();
::CloseHandle(in_queue_);
delete this;
}
@@ -255,7 +239,9 @@
// and WPARAM is 32bits in 32bit builds. Otherwise, we could pass the
// task pointer and timestamp as LPARAM and WPARAM.
auto* task_info = new DelayedTaskInfo(milliseconds, std::move(task));
- if (!::PostThreadMessage(thread_.GetThreadRef(), WM_QUEUE_DELAYED_TASK, 0,
+ RTC_CHECK(thread_.GetHandle() != absl::nullopt);
+ if (!::PostThreadMessage(GetThreadId(*thread_.GetHandle()),
+ WM_QUEUE_DELAYED_TASK, 0,
reinterpret_cast<LPARAM>(task_info))) {
delete task_info;
}
@@ -277,11 +263,6 @@
}
}
-// static
-void TaskQueueWin::ThreadMain(void* context) {
- static_cast<TaskQueueWin*>(context)->RunThreadMain();
-}
-
void TaskQueueWin::RunThreadMain() {
CurrentTaskQueueSetter set_current(this);
HANDLE handles[2] = {*timer_.event_for_wait(), in_queue_};