Reland of Make the default ctor of rtc::Thread, protected
This is a partial re-land. The change doesn't make the default Thread ctor protected anymore but it does mark it as deprecated and updates all use of it in WebRTC.
Original issue's description:
Make the default ctor of rtc::Thread, protected.
The goal is to force use of Thread::Create or Thread::CreateWithSocketServer.
The default constructor constructs a 'default' socket server, which is usually a 'physical' socket server, but not always. Not every instance of Thread actually needs to have network support, so it's better to have this be explicit instead of unknowingly instantiate one.
BUG=none
Review-Url: https://codereview.webrtc.org/2977953002
Cr-Commit-Position: refs/heads/master@{#19031}
diff --git a/webrtc/rtc_base/thread_unittest.cc b/webrtc/rtc_base/thread_unittest.cc
index db7d038..a8c20d1 100644
--- a/webrtc/rtc_base/thread_unittest.cc
+++ b/webrtc/rtc_base/thread_unittest.cc
@@ -14,6 +14,7 @@
#include "webrtc/rtc_base/asyncudpsocket.h"
#include "webrtc/rtc_base/event.h"
#include "webrtc/rtc_base/gunit.h"
+#include "webrtc/rtc_base/nullsocketserver.h"
#include "webrtc/rtc_base/physicalsocketserver.h"
#include "webrtc/rtc_base/sigslot.h"
#include "webrtc/rtc_base/socketaddress.h"
@@ -106,7 +107,8 @@
class CustomThread : public rtc::Thread {
public:
- CustomThread() {}
+ CustomThread()
+ : Thread(std::unique_ptr<SocketServer>(new rtc::NullSocketServer())) {}
virtual ~CustomThread() { Stop(); }
bool Start() { return false; }
@@ -124,8 +126,8 @@
class SignalWhenDestroyedThread : public Thread {
public:
SignalWhenDestroyedThread(Event* event)
- : event_(event) {
- }
+ : Thread(std::unique_ptr<SocketServer>(new NullSocketServer())),
+ event_(event) {}
virtual ~SignalWhenDestroyedThread() {
Stop();
@@ -195,24 +197,24 @@
const SocketAddress addr("127.0.0.1", 0);
// Create the messaging client on its own thread.
- Thread th1;
- Socket* socket = th1.socketserver()->CreateAsyncSocket(addr.family(),
- SOCK_DGRAM);
- MessageClient msg_client(&th1, socket);
+ auto th1 = Thread::CreateWithSocketServer();
+ Socket* socket =
+ th1->socketserver()->CreateAsyncSocket(addr.family(), SOCK_DGRAM);
+ MessageClient msg_client(th1.get(), socket);
// Create the socket client on its own thread.
- Thread th2;
+ auto th2 = Thread::CreateWithSocketServer();
AsyncSocket* asocket =
- th2.socketserver()->CreateAsyncSocket(addr.family(), SOCK_DGRAM);
- SocketClient sock_client(asocket, addr, &th1, &msg_client);
+ th2->socketserver()->CreateAsyncSocket(addr.family(), SOCK_DGRAM);
+ SocketClient sock_client(asocket, addr, th1.get(), &msg_client);
socket->Connect(sock_client.address());
- th1.Start();
- th2.Start();
+ th1->Start();
+ th2->Start();
// Get the messages started.
- th1.PostDelayed(RTC_FROM_HERE, 100, &msg_client, 0, new TestMessage(1));
+ th1->PostDelayed(RTC_FROM_HERE, 100, &msg_client, 0, new TestMessage(1));
// Give the clients a little while to run.
// Messages will be processed at 100, 300, 500, 700, 900.
@@ -221,9 +223,9 @@
// Stop the sending client. Give the receiver a bit longer to run, in case
// it is running on a machine that is under load (e.g. the build machine).
- th1.Stop();
+ th1->Stop();
th_main->ProcessMessages(200);
- th2.Stop();
+ th2->Stop();
// Make sure the results were correct
EXPECT_EQ(5, msg_client.count);
@@ -236,23 +238,19 @@
// There's no easy way to verify the name was set properly at this time.
TEST(ThreadTest, Names) {
// Default name
- Thread *thread;
- thread = new Thread();
+ auto thread = Thread::CreateWithSocketServer();
EXPECT_TRUE(thread->Start());
thread->Stop();
- delete thread;
- thread = new Thread();
// Name with no object parameter
+ thread = Thread::CreateWithSocketServer();
EXPECT_TRUE(thread->SetName("No object", nullptr));
EXPECT_TRUE(thread->Start());
thread->Stop();
- delete thread;
// Really long name
- thread = new Thread();
+ thread = Thread::CreateWithSocketServer();
EXPECT_TRUE(thread->SetName("Abcdefghijklmnopqrstuvwxyz1234567890", this));
EXPECT_TRUE(thread->Start());
thread->Stop();
- delete thread;
}
TEST(ThreadTest, Wrap) {
@@ -270,21 +268,21 @@
TEST(ThreadTest, Invoke) {
// Create and start the thread.
- Thread thread;
- thread.Start();
+ auto thread = Thread::CreateWithSocketServer();
+ thread->Start();
// Try calling functors.
- EXPECT_EQ(42, thread.Invoke<int>(RTC_FROM_HERE, FunctorA()));
+ EXPECT_EQ(42, thread->Invoke<int>(RTC_FROM_HERE, FunctorA()));
AtomicBool called;
FunctorB f2(&called);
- thread.Invoke<void>(RTC_FROM_HERE, f2);
+ thread->Invoke<void>(RTC_FROM_HERE, f2);
EXPECT_TRUE(called.get());
// Try calling bare functions.
struct LocalFuncs {
static int Func1() { return 999; }
static void Func2() {}
};
- EXPECT_EQ(999, thread.Invoke<int>(RTC_FROM_HERE, &LocalFuncs::Func1));
- thread.Invoke<void>(RTC_FROM_HERE, &LocalFuncs::Func2);
+ EXPECT_EQ(999, thread->Invoke<int>(RTC_FROM_HERE, &LocalFuncs::Func1));
+ thread->Invoke<void>(RTC_FROM_HERE, &LocalFuncs::Func2);
}
// Verifies that two threads calling Invoke on each other at the same time does
@@ -294,8 +292,8 @@
Thread* current_thread = Thread::Current();
ASSERT_TRUE(current_thread != nullptr);
- Thread other_thread;
- other_thread.Start();
+ auto other_thread = Thread::CreateWithSocketServer();
+ other_thread->Start();
struct LocalFuncs {
static void Set(bool* out) { *out = true; }
@@ -305,7 +303,7 @@
};
bool called = false;
- other_thread.Invoke<void>(
+ other_thread->Invoke<void>(
RTC_FROM_HERE, Bind(&LocalFuncs::InvokeSet, current_thread, &called));
EXPECT_TRUE(called);
@@ -317,9 +315,10 @@
TEST(ThreadTest, ThreeThreadsInvoke) {
AutoThread thread;
Thread* thread_a = Thread::Current();
- Thread thread_b, thread_c;
- thread_b.Start();
- thread_c.Start();
+ auto thread_b = Thread::CreateWithSocketServer();
+ auto thread_c = Thread::CreateWithSocketServer();
+ thread_b->Start();
+ thread_c->Start();
class LockedBool {
public:
@@ -377,9 +376,9 @@
// Start the sequence A --(invoke)--> B --(async invoke)--> C --(invoke)--> A.
// Thread B returns when C receives the call and C should be blocked until A
// starts to process messages.
- thread_b.Invoke<void>(RTC_FROM_HERE,
- Bind(&LocalFuncs::AsyncInvokeSetAndWait, &invoker,
- &thread_c, thread_a, &thread_a_called));
+ thread_b->Invoke<void>(RTC_FROM_HERE,
+ Bind(&LocalFuncs::AsyncInvokeSetAndWait, &invoker,
+ thread_c.get(), thread_a, &thread_a_called));
EXPECT_FALSE(thread_a_called.Get());
EXPECT_TRUE_WAIT(thread_a_called.Get(), 2000);
@@ -406,9 +405,9 @@
};
TEST(ThreadTest, SetNameOnSignalQueueDestroyed) {
- Thread* thread1 = new Thread();
- SetNameOnSignalQueueDestroyedTester tester1(thread1);
- delete thread1;
+ auto thread1 = Thread::CreateWithSocketServer();
+ SetNameOnSignalQueueDestroyedTester tester1(thread1.get());
+ thread1.reset();
Thread* thread2 = new AutoThread();
SetNameOnSignalQueueDestroyedTester tester2(thread2);
@@ -438,12 +437,13 @@
TEST_F(AsyncInvokeTest, FireAndForget) {
AsyncInvoker invoker;
// Create and start the thread.
- Thread thread;
- thread.Start();
+ auto thread = Thread::CreateWithSocketServer();
+ thread->Start();
// Try calling functor.
AtomicBool called;
- invoker.AsyncInvoke<void>(RTC_FROM_HERE, &thread, FunctorB(&called));
+ invoker.AsyncInvoke<void>(RTC_FROM_HERE, thread.get(), FunctorB(&called));
EXPECT_TRUE_WAIT(called.get(), kWaitTimeout);
+ thread->Stop();
}
TEST_F(AsyncInvokeTest, KillInvokerDuringExecute) {
@@ -454,12 +454,12 @@
Event functor_continue(false, false);
Event functor_finished(false, false);
- Thread thread;
- thread.Start();
+ auto thread = Thread::CreateWithSocketServer();
+ thread->Start();
volatile bool invoker_destroyed = false;
{
AsyncInvoker invoker;
- invoker.AsyncInvoke<void>(RTC_FROM_HERE, &thread,
+ invoker.AsyncInvoke<void>(RTC_FROM_HERE, thread.get(),
[&functor_started, &functor_continue,
&functor_finished, &invoker_destroyed] {
functor_started.Set();
@@ -550,7 +550,7 @@
// Test that we can call AsyncInvoke<void>() after the thread died.
TEST_F(GuardedAsyncInvokeTest, KillThreadFireAndForget) {
// Create and start the thread.
- std::unique_ptr<Thread> thread(new Thread());
+ std::unique_ptr<Thread> thread(Thread::Create());
thread->Start();
std::unique_ptr<GuardedAsyncInvoker> invoker;
// Create the invoker on |thread|.
