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/platform_thread.cc b/rtc_base/platform_thread.cc
index c5f3bc3..6d369d7 100644
--- a/rtc_base/platform_thread.cc
+++ b/rtc_base/platform_thread.cc
@@ -10,32 +10,37 @@
#include "rtc_base/platform_thread.h"
+#include <algorithm>
#include <memory>
#if !defined(WEBRTC_WIN)
#include <sched.h>
#endif
-#include <stdint.h>
-#include <time.h>
-#include <algorithm>
-
-#include "absl/memory/memory.h"
#include "rtc_base/checks.h"
namespace rtc {
-
namespace {
-struct ThreadStartData {
- ThreadRunFunction run_function;
- void* obj;
- std::string thread_name;
- ThreadPriority priority;
-};
+
+#if defined(WEBRTC_WIN)
+int Win32PriorityFromThreadPriority(ThreadPriority priority) {
+ switch (priority) {
+ case ThreadPriority::kLow:
+ return THREAD_PRIORITY_BELOW_NORMAL;
+ case ThreadPriority::kNormal:
+ return THREAD_PRIORITY_NORMAL;
+ case ThreadPriority::kHigh:
+ return THREAD_PRIORITY_ABOVE_NORMAL;
+ case ThreadPriority::kRealtime:
+ return THREAD_PRIORITY_TIME_CRITICAL;
+ }
+}
+#endif
bool SetPriority(ThreadPriority priority) {
#if defined(WEBRTC_WIN)
- return SetThreadPriority(GetCurrentThread(), priority) != FALSE;
+ return SetThreadPriority(GetCurrentThread(),
+ Win32PriorityFromThreadPriority(priority)) != FALSE;
#elif defined(__native_client__) || defined(WEBRTC_FUCHSIA)
// Setting thread priorities is not supported in NaCl or Fuchsia.
return true;
@@ -59,21 +64,18 @@
const int top_prio = max_prio - 1;
const int low_prio = min_prio + 1;
switch (priority) {
- case kLowPriority:
+ case ThreadPriority::kLow:
param.sched_priority = low_prio;
break;
- case kNormalPriority:
+ case ThreadPriority::kNormal:
// The -1 ensures that the kHighPriority is always greater or equal to
// kNormalPriority.
param.sched_priority = (low_prio + top_prio - 1) / 2;
break;
- case kHighPriority:
+ case ThreadPriority::kHigh:
param.sched_priority = std::max(top_prio - 2, low_prio);
break;
- case kHighestPriority:
- param.sched_priority = std::max(top_prio - 1, low_prio);
- break;
- case kRealtimePriority:
+ case ThreadPriority::kRealtime:
param.sched_priority = top_prio;
break;
}
@@ -81,124 +83,129 @@
#endif // defined(WEBRTC_WIN)
}
-void RunPlatformThread(std::unique_ptr<ThreadStartData> data) {
- rtc::SetCurrentThreadName(data->thread_name.c_str());
- data->thread_name.clear();
- SetPriority(data->priority);
- data->run_function(data->obj);
-}
-
#if defined(WEBRTC_WIN)
-DWORD WINAPI StartThread(void* param) {
+DWORD WINAPI RunPlatformThread(void* param) {
// The GetLastError() function only returns valid results when it is called
// after a Win32 API function that returns a "failed" result. A crash dump
// contains the result from GetLastError() and to make sure it does not
// falsely report a Windows error we call SetLastError here.
::SetLastError(ERROR_SUCCESS);
- RunPlatformThread(absl::WrapUnique(static_cast<ThreadStartData*>(param)));
+ auto function = static_cast<std::function<void()>*>(param);
+ (*function)();
+ delete function;
return 0;
}
#else
-void* StartThread(void* param) {
- RunPlatformThread(absl::WrapUnique(static_cast<ThreadStartData*>(param)));
+void* RunPlatformThread(void* param) {
+ auto function = static_cast<std::function<void()>*>(param);
+ (*function)();
+ delete function;
return 0;
}
#endif // defined(WEBRTC_WIN)
} // namespace
-PlatformThread::PlatformThread(ThreadRunFunction func,
- void* obj,
- absl::string_view thread_name,
- ThreadAttributes attributes)
- : run_function_(func),
- attributes_(attributes),
- obj_(obj),
- name_(thread_name) {
- RTC_DCHECK(func);
- RTC_DCHECK(!name_.empty());
- // TODO(tommi): Consider lowering the limit to 15 (limit on Linux).
- RTC_DCHECK(name_.length() < 64);
+PlatformThread::PlatformThread(Handle handle, bool joinable)
+ : handle_(handle), joinable_(joinable) {}
+
+PlatformThread::PlatformThread(PlatformThread&& rhs)
+ : handle_(rhs.handle_), joinable_(rhs.joinable_) {
+ rhs.handle_ = absl::nullopt;
+}
+
+PlatformThread& PlatformThread::operator=(PlatformThread&& rhs) {
+ Finalize();
+ handle_ = rhs.handle_;
+ joinable_ = rhs.joinable_;
+ rhs.handle_ = absl::nullopt;
+ return *this;
}
PlatformThread::~PlatformThread() {
- RTC_DCHECK_RUN_ON(&thread_checker_);
- RTC_DCHECK(!thread_);
-#if defined(WEBRTC_WIN)
- RTC_DCHECK(!thread_id_);
-#endif // defined(WEBRTC_WIN)
+ Finalize();
}
-void PlatformThread::Start() {
- RTC_DCHECK_RUN_ON(&thread_checker_);
- RTC_DCHECK(!thread_) << "Thread already started?";
- ThreadStartData* data =
- new ThreadStartData{run_function_, obj_, name_, attributes_.priority};
+PlatformThread PlatformThread::SpawnJoinable(
+ std::function<void()> thread_function,
+ absl::string_view name,
+ ThreadAttributes attributes) {
+ return SpawnThread(std::move(thread_function), name, attributes,
+ /*joinable=*/true);
+}
+
+PlatformThread PlatformThread::SpawnDetached(
+ std::function<void()> thread_function,
+ absl::string_view name,
+ ThreadAttributes attributes) {
+ return SpawnThread(std::move(thread_function), name, attributes,
+ /*joinable=*/false);
+}
+
+absl::optional<PlatformThread::Handle> PlatformThread::GetHandle() const {
+ return handle_;
+}
+
+#if defined(WEBRTC_WIN)
+bool PlatformThread::QueueAPC(PAPCFUNC function, ULONG_PTR data) {
+ RTC_DCHECK(handle_.has_value());
+ return handle_.has_value() ? QueueUserAPC(function, *handle_, data) != FALSE
+ : false;
+}
+#endif
+
+void PlatformThread::Finalize() {
+ if (!handle_.has_value())
+ return;
+#if defined(WEBRTC_WIN)
+ if (joinable_)
+ WaitForSingleObject(*handle_, INFINITE);
+ CloseHandle(*handle_);
+#else
+ if (joinable_)
+ RTC_CHECK_EQ(0, pthread_join(*handle_, nullptr));
+#endif
+ handle_ = absl::nullopt;
+}
+
+PlatformThread PlatformThread::SpawnThread(
+ std::function<void()> thread_function,
+ absl::string_view name,
+ ThreadAttributes attributes,
+ bool joinable) {
+ RTC_DCHECK(thread_function);
+ RTC_DCHECK(!name.empty());
+ // TODO(tommi): Consider lowering the limit to 15 (limit on Linux).
+ RTC_DCHECK(name.length() < 64);
+ auto start_thread_function_ptr =
+ new std::function<void()>([thread_function = std::move(thread_function),
+ name = std::string(name), attributes] {
+ rtc::SetCurrentThreadName(name.c_str());
+ SetPriority(attributes.priority);
+ thread_function();
+ });
#if defined(WEBRTC_WIN)
// See bug 2902 for background on STACK_SIZE_PARAM_IS_A_RESERVATION.
// Set the reserved stack stack size to 1M, which is the default on Windows
// and Linux.
- thread_ = ::CreateThread(nullptr, 1024 * 1024, &StartThread, data,
- STACK_SIZE_PARAM_IS_A_RESERVATION, &thread_id_);
- RTC_CHECK(thread_) << "CreateThread failed";
- RTC_DCHECK(thread_id_);
+ DWORD thread_id = 0;
+ PlatformThread::Handle handle = ::CreateThread(
+ nullptr, 1024 * 1024, &RunPlatformThread, start_thread_function_ptr,
+ STACK_SIZE_PARAM_IS_A_RESERVATION, &thread_id);
+ RTC_CHECK(handle) << "CreateThread failed";
#else
pthread_attr_t attr;
pthread_attr_init(&attr);
// Set the stack stack size to 1M.
pthread_attr_setstacksize(&attr, 1024 * 1024);
- pthread_attr_setdetachstate(&attr, attributes_.joinable
- ? PTHREAD_CREATE_JOINABLE
- : PTHREAD_CREATE_DETACHED);
- RTC_CHECK_EQ(0, pthread_create(&thread_, &attr, &StartThread, data));
+ pthread_attr_setdetachstate(
+ &attr, joinable ? PTHREAD_CREATE_JOINABLE : PTHREAD_CREATE_DETACHED);
+ PlatformThread::Handle handle;
+ RTC_CHECK_EQ(0, pthread_create(&handle, &attr, &RunPlatformThread,
+ start_thread_function_ptr));
pthread_attr_destroy(&attr);
#endif // defined(WEBRTC_WIN)
+ return PlatformThread(handle, joinable);
}
-bool PlatformThread::IsRunning() const {
- RTC_DCHECK_RUN_ON(&thread_checker_);
-#if defined(WEBRTC_WIN)
- return thread_ != nullptr;
-#else
- return thread_ != 0;
-#endif // defined(WEBRTC_WIN)
-}
-
-PlatformThreadRef PlatformThread::GetThreadRef() const {
-#if defined(WEBRTC_WIN)
- return thread_id_;
-#else
- return thread_;
-#endif // defined(WEBRTC_WIN)
-}
-
-void PlatformThread::Stop() {
- RTC_DCHECK_RUN_ON(&thread_checker_);
- if (!IsRunning())
- return;
-
-#if defined(WEBRTC_WIN)
- if (attributes_.joinable) {
- WaitForSingleObject(thread_, INFINITE);
- }
- CloseHandle(thread_);
- thread_ = nullptr;
- thread_id_ = 0;
-#else
- if (attributes_.joinable) {
- RTC_CHECK_EQ(0, pthread_join(thread_, nullptr));
- }
- thread_ = 0;
-#endif // defined(WEBRTC_WIN)
-}
-
-#if defined(WEBRTC_WIN)
-bool PlatformThread::QueueAPC(PAPCFUNC function, ULONG_PTR data) {
- RTC_DCHECK_RUN_ON(&thread_checker_);
- RTC_DCHECK(IsRunning());
-
- return QueueUserAPC(function, thread_, data) != FALSE;
-}
-#endif
-
} // namespace rtc