commit 793bac569fdf1be16cbf24d7871d20d00bbec81b
author Guido Urdaneta <guidou@webrtc.org> Thu May 06 13:12:47 2021 +0000
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu May 06 14:22:57 2021 +0000
tree f6f1ee3d345221f39507bba299b47a5123752be1
parent a6983c6ea20fdae57945e8aeab45bb99d7e7b23d

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}

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 b77d867..fcd9c9b 100644
--- a/sdk/android/native_unittests/stacktrace/stacktrace_unittest.cc
+++ b/sdk/android/native_unittests/stacktrace/stacktrace_unittest.cc
@@ -153,24 +153,28 @@
   }
 };
 
+// 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.
-  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");
+  rtc::PlatformThread thread(&ThreadFunction, &params, "StacktraceTest");
+  thread.Start();
 
   // Wait until the thread has entered the deadlock region, and take a very
   // brief nap to give it time to reach the actual deadlock.
@@ -194,6 +198,8 @@
       << 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 {
@@ -253,9 +259,13 @@
 
   // Start a thread that waits for an event.
   rtc::Event ev;
-  auto thread = rtc::PlatformThread::SpawnJoinable(
-      [&ev] { ev.Wait(rtc::Event::kForever); },
-      "TestRtcEventDeadlockDetection");
+  rtc::PlatformThread thread(
+      [](void* arg) {
+        auto* ev = static_cast<rtc::Event*>(arg);
+        ev->Wait(rtc::Event::kForever);
+      },
+      &ev, "TestRtcEventDeadlockDetection");
+  thread.Start();
 
   // The message should appear after 3 sec. We'll wait up to 10 sec in an
   // attempt to not be flaky.
@@ -263,7 +273,7 @@
 
   // Unblock the thread and shut it down.
   ev.Set();
-  thread.Finalize();
+  thread.Stop();
   rtc::LogMessage::RemoveLogToStream(&sink);
 }