commit ad5037b4a81cf97c54c5b15fda8ee898f1827e58
author Markus Handell <handellm@webrtc.org> Fri May 07 15:02:36 2021 +0200
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri May 07 14:14:43 2021 +0000
tree 853f8ace9e654c4677a0dbb3df268cdf143a255d
parent f95536dd5a39e7bef6ddcd2f507a63064e07c8cc

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}

sdk/android/native_unittests/stacktrace/stacktrace_unittest.cc

diff --git a/sdk/android/native_unittests/stacktrace/stacktrace_unittest.cc b/sdk/android/native_unittests/stacktrace/stacktrace_unittest.cc
index fcd9c9b..b77d867 100644
--- a/sdk/android/native_unittests/stacktrace/stacktrace_unittest.cc
+++ b/sdk/android/native_unittests/stacktrace/stacktrace_unittest.cc
@@ -153,28 +153,24 @@
   }
 };
 
-// This is the function that is exectued by the thread that will deadlock and
-// have its stacktrace captured.
-void ThreadFunction(void* void_params) {
-  ThreadParams* params = static_cast<ThreadParams*>(void_params);
-  params->tid = gettid();
-
-  params->deadlock_region_start_address = GetCurrentRelativeExecutionAddress();
-  params->deadlock_start_event.Set();
-  params->deadlock_impl->Deadlock();
-  params->deadlock_region_end_address = GetCurrentRelativeExecutionAddress();
-
-  params->deadlock_done_event.Set();
-}
-
 void TestStacktrace(std::unique_ptr<DeadlockInterface> deadlock_impl) {
   // Set params that will be sent to other thread.
   ThreadParams params;
   params.deadlock_impl = deadlock_impl.get();
 
   // Spawn thread.
-  rtc::PlatformThread thread(&ThreadFunction, &params, "StacktraceTest");
-  thread.Start();
+  auto thread = rtc::PlatformThread::SpawnJoinable(
+      [&params] {
+        params.tid = gettid();
+        params.deadlock_region_start_address =
+            GetCurrentRelativeExecutionAddress();
+        params.deadlock_start_event.Set();
+        params.deadlock_impl->Deadlock();
+        params.deadlock_region_end_address =
+            GetCurrentRelativeExecutionAddress();
+        params.deadlock_done_event.Set();
+      },
+      "StacktraceTest");
 
   // Wait until the thread has entered the deadlock region, and take a very
   // brief nap to give it time to reach the actual deadlock.
@@ -198,8 +194,6 @@
       << rtc::ToHex(params.deadlock_region_start_address) << ", "
       << rtc::ToHex(params.deadlock_region_end_address)
       << "] not contained in: " << StackTraceToString(stack_trace);
-
-  thread.Stop();
 }
 
 class LookoutLogSink final : public rtc::LogSink {
@@ -259,13 +253,9 @@
 
   // Start a thread that waits for an event.
   rtc::Event ev;
-  rtc::PlatformThread thread(
-      [](void* arg) {
-        auto* ev = static_cast<rtc::Event*>(arg);
-        ev->Wait(rtc::Event::kForever);
-      },
-      &ev, "TestRtcEventDeadlockDetection");
-  thread.Start();
+  auto thread = rtc::PlatformThread::SpawnJoinable(
+      [&ev] { ev.Wait(rtc::Event::kForever); },
+      "TestRtcEventDeadlockDetection");
 
   // The message should appear after 3 sec. We'll wait up to 10 sec in an
   // attempt to not be flaky.
@@ -273,7 +263,7 @@
 
   // Unblock the thread and shut it down.
   ev.Set();
-  thread.Stop();
+  thread.Finalize();
   rtc::LogMessage::RemoveLogToStream(&sink);
 }