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}
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();