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_unittest.cc b/rtc_base/platform_thread_unittest.cc
index d09772f..0da822c 100644
--- a/rtc_base/platform_thread_unittest.cc
+++ b/rtc_base/platform_thread_unittest.cc
@@ -10,69 +10,73 @@
#include "rtc_base/platform_thread.h"
+#include "absl/types/optional.h"
#include "rtc_base/event.h"
#include "system_wrappers/include/sleep.h"
#include "test/gmock.h"
namespace rtc {
-namespace {
-void NullRunFunction(void* obj) {}
-
-// Function that sets a boolean.
-void SetFlagRunFunction(void* obj) {
- bool* obj_as_bool = static_cast<bool*>(obj);
- *obj_as_bool = true;
+TEST(PlatformThreadTest, DefaultConstructedIsEmpty) {
+ PlatformThread thread;
+ EXPECT_EQ(thread.GetHandle(), absl::nullopt);
+ EXPECT_TRUE(thread.empty());
}
-void StdFunctionRunFunction(void* obj) {
- std::function<void()>* fun = static_cast<std::function<void()>*>(obj);
- (*fun)();
+TEST(PlatformThreadTest, StartFinalize) {
+ PlatformThread thread = PlatformThread::SpawnJoinable([] {}, "1");
+ EXPECT_NE(thread.GetHandle(), absl::nullopt);
+ EXPECT_FALSE(thread.empty());
+ thread.Finalize();
+ EXPECT_TRUE(thread.empty());
+ thread = PlatformThread::SpawnDetached([] {}, "2");
+ EXPECT_FALSE(thread.empty());
+ thread.Finalize();
+ EXPECT_TRUE(thread.empty());
}
-} // namespace
-
-TEST(PlatformThreadTest, StartStop) {
- PlatformThread thread(&NullRunFunction, nullptr, "PlatformThreadTest");
- EXPECT_TRUE(thread.name() == "PlatformThreadTest");
- EXPECT_TRUE(thread.GetThreadRef() == 0);
- thread.Start();
- EXPECT_TRUE(thread.GetThreadRef() != 0);
- thread.Stop();
- EXPECT_TRUE(thread.GetThreadRef() == 0);
+TEST(PlatformThreadTest, MovesEmpty) {
+ PlatformThread thread1;
+ PlatformThread thread2 = std::move(thread1);
+ EXPECT_TRUE(thread1.empty());
+ EXPECT_TRUE(thread2.empty());
}
-TEST(PlatformThreadTest, StartStop2) {
- PlatformThread thread1(&NullRunFunction, nullptr, "PlatformThreadTest1");
- PlatformThread thread2(&NullRunFunction, nullptr, "PlatformThreadTest2");
- EXPECT_TRUE(thread1.GetThreadRef() == thread2.GetThreadRef());
- thread1.Start();
- thread2.Start();
- EXPECT_TRUE(thread1.GetThreadRef() != thread2.GetThreadRef());
- thread2.Stop();
- thread1.Stop();
+TEST(PlatformThreadTest, MovesHandles) {
+ PlatformThread thread1 = PlatformThread::SpawnJoinable([] {}, "1");
+ PlatformThread thread2 = std::move(thread1);
+ EXPECT_TRUE(thread1.empty());
+ EXPECT_FALSE(thread2.empty());
+ thread1 = PlatformThread::SpawnDetached([] {}, "2");
+ thread2 = std::move(thread1);
+ EXPECT_TRUE(thread1.empty());
+ EXPECT_FALSE(thread2.empty());
+}
+
+TEST(PlatformThreadTest,
+ TwoThreadHandlesAreDifferentWhenStartedAndEqualWhenJoined) {
+ PlatformThread thread1 = PlatformThread();
+ PlatformThread thread2 = PlatformThread();
+ EXPECT_EQ(thread1.GetHandle(), thread2.GetHandle());
+ thread1 = PlatformThread::SpawnJoinable([] {}, "1");
+ thread2 = PlatformThread::SpawnJoinable([] {}, "2");
+ EXPECT_NE(thread1.GetHandle(), thread2.GetHandle());
+ thread1.Finalize();
+ EXPECT_NE(thread1.GetHandle(), thread2.GetHandle());
+ thread2.Finalize();
+ EXPECT_EQ(thread1.GetHandle(), thread2.GetHandle());
}
TEST(PlatformThreadTest, RunFunctionIsCalled) {
bool flag = false;
- PlatformThread thread(&SetFlagRunFunction, &flag, "RunFunctionIsCalled");
- thread.Start();
-
- // At this point, the flag may be either true or false.
- thread.Stop();
-
- // We expect the thread to have run at least once.
+ PlatformThread::SpawnJoinable([&] { flag = true; }, "T");
EXPECT_TRUE(flag);
}
TEST(PlatformThreadTest, JoinsThread) {
// This test flakes if there are problems with the join implementation.
- EXPECT_TRUE(ThreadAttributes().joinable);
rtc::Event event;
- std::function<void()> thread_function = [&] { event.Set(); };
- PlatformThread thread(&StdFunctionRunFunction, &thread_function, "T");
- thread.Start();
- thread.Stop();
+ PlatformThread::SpawnJoinable([&] { event.Set(); }, "T");
EXPECT_TRUE(event.Wait(/*give_up_after_ms=*/0));
}
@@ -83,18 +87,14 @@
rtc::Event thread_started;
rtc::Event thread_continue;
rtc::Event thread_exiting;
- std::function<void()> thread_function = [&] {
- thread_started.Set();
- thread_continue.Wait(Event::kForever);
- flag = true;
- thread_exiting.Set();
- };
- {
- PlatformThread thread(&StdFunctionRunFunction, &thread_function, "T",
- ThreadAttributes().SetDetached());
- thread.Start();
- thread.Stop();
- }
+ PlatformThread::SpawnDetached(
+ [&] {
+ thread_started.Set();
+ thread_continue.Wait(Event::kForever);
+ flag = true;
+ thread_exiting.Set();
+ },
+ "T");
thread_started.Wait(Event::kForever);
EXPECT_FALSE(flag);
thread_continue.Set();