Make mojo::RunLoop just use pthreads.
In particular, it just uses pthreads TLS stuff directly along with
pthread_once(), which allows RunLoop::{SetUp,TearDown}() to be removed.
This will allow us to make the "standalone" Environment like the
"Chromium"/base-using Environment, i.e., not require instantiation. This
will make things like ApplicationRunner more sane (since it currently
has to mix two things: instantiating the Environment -- which you need
to do once per "process", i.e., Mojo application binary instantiation --
and instantiating a RunLoop -- which you need to do once per thread).
(Really, ApplicationRunner should just be a function, but that's another
story.)
Also note that I can't just use C++11 TLS (I tried:
1c4b90d52b54811cfeeb7d72fd63ee3c33006866), since it's not supported on
iOS (which made Flutter sad). pthreads is OK though (including, AFAICT,
pthread_once()).
R=vardhan@google.com
Review URL: https://codereview.chromium.org/1996703002 .
diff --git a/mojo/public/cpp/environment/lib/environment.cc b/mojo/public/cpp/environment/lib/environment.cc
index 8224f1d..8224970 100644
--- a/mojo/public/cpp/environment/lib/environment.cc
+++ b/mojo/public/cpp/environment/lib/environment.cc
@@ -25,8 +25,6 @@
: &internal::kDefaultAsyncWaiter;
g_default_logger =
default_logger ? default_logger : &internal::kDefaultLogger;
-
- RunLoop::SetUp();
}
} // namespace
@@ -41,8 +39,6 @@
}
Environment::~Environment() {
- RunLoop::TearDown();
-
// TODO(vtl): Maybe we should allow nesting, and restore previous default
// async waiters and loggers?
g_default_async_waiter = nullptr;
diff --git a/mojo/public/cpp/utility/BUILD.gn b/mojo/public/cpp/utility/BUILD.gn
index d47d5c4..dd2c240 100644
--- a/mojo/public/cpp/utility/BUILD.gn
+++ b/mojo/public/cpp/utility/BUILD.gn
@@ -7,9 +7,6 @@
mojo_sdk_source_set("utility") {
sources = [
"lib/run_loop.cc",
- "lib/thread_local.h",
- "lib/thread_local_posix.cc",
- "lib/thread_local_win.cc",
"run_loop.h",
"run_loop_handler.h",
]
diff --git a/mojo/public/cpp/utility/lib/run_loop.cc b/mojo/public/cpp/utility/lib/run_loop.cc
index f68ad79..1fde95c 100644
--- a/mojo/public/cpp/utility/lib/run_loop.cc
+++ b/mojo/public/cpp/utility/lib/run_loop.cc
@@ -5,22 +5,44 @@
#include "mojo/public/cpp/utility/run_loop.h"
#include <assert.h>
+#include <pthread.h>
#include <algorithm>
#include <vector>
+#include "mojo/public/c/system/macros.h"
#include "mojo/public/cpp/system/time.h"
#include "mojo/public/cpp/system/wait.h"
-#include "mojo/public/cpp/utility/lib/thread_local.h"
#include "mojo/public/cpp/utility/run_loop_handler.h"
namespace mojo {
namespace {
-internal::ThreadLocalPointer<RunLoop> current_run_loop;
-
const MojoTimeTicks kInvalidTimeTicks = static_cast<MojoTimeTicks>(0);
+pthread_key_t g_current_run_loop_key;
+
+// Ensures that the "current run loop" functionality is available (i.e., that we
+// have a TLS slot).
+void EnsureCurrentRunLoopInitialized() {
+ static pthread_once_t current_run_loop_key_once = PTHREAD_ONCE_INIT;
+ int error = pthread_once(¤t_run_loop_key_once, []() {
+ int error = pthread_key_create(&g_current_run_loop_key, nullptr);
+ MOJO_ALLOW_UNUSED_LOCAL(error);
+ assert(!error);
+ });
+ MOJO_ALLOW_UNUSED_LOCAL(error);
+ assert(!error);
+}
+
+void SetCurrentRunLoop(RunLoop* run_loop) {
+ EnsureCurrentRunLoopInitialized();
+
+ int error = pthread_setspecific(g_current_run_loop_key, run_loop);
+ MOJO_ALLOW_UNUSED_LOCAL(error);
+ assert(!error);
+}
+
// State needed for one iteration of WaitMany().
struct WaitState {
std::vector<Handle> handles;
@@ -38,29 +60,19 @@
RunLoop::RunLoop()
: run_state_(nullptr), next_handler_id_(0), next_sequence_number_(0) {
assert(!current());
- current_run_loop.Set(this);
+ SetCurrentRunLoop(this);
}
RunLoop::~RunLoop() {
assert(current() == this);
NotifyHandlers(MOJO_RESULT_ABORTED, IGNORE_DEADLINE);
- current_run_loop.Set(nullptr);
-}
-
-// static
-void RunLoop::SetUp() {
- current_run_loop.Allocate();
-}
-
-// static
-void RunLoop::TearDown() {
- assert(!current());
- current_run_loop.Free();
+ SetCurrentRunLoop(nullptr);
}
// static
RunLoop* RunLoop::current() {
- return current_run_loop.Get();
+ EnsureCurrentRunLoopInitialized();
+ return static_cast<RunLoop*>(pthread_getspecific(g_current_run_loop_key));
}
void RunLoop::AddHandler(RunLoopHandler* handler,
diff --git a/mojo/public/cpp/utility/lib/thread_local.h b/mojo/public/cpp/utility/lib/thread_local.h
deleted file mode 100644
index f5461ee..0000000
--- a/mojo/public/cpp/utility/lib/thread_local.h
+++ /dev/null
@@ -1,54 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef MOJO_PUBLIC_CPP_UTILITY_LIB_THREAD_LOCAL_H_
-#define MOJO_PUBLIC_CPP_UTILITY_LIB_THREAD_LOCAL_H_
-
-#ifndef _WIN32
-#include <pthread.h>
-#endif
-
-#include "mojo/public/cpp/system/macros.h"
-
-namespace mojo {
-namespace internal {
-
-// Helper functions that abstract the cross-platform APIs.
-struct ThreadLocalPlatform {
-#ifdef _WIN32
- typedef unsigned long SlotType;
-#else
- typedef pthread_key_t SlotType;
-#endif
-
- static void AllocateSlot(SlotType* slot);
- static void FreeSlot(SlotType slot);
- static void* GetValueFromSlot(SlotType slot);
- static void SetValueInSlot(SlotType slot, void* value);
-};
-
-// This class is intended to be statically allocated.
-template <typename P>
-class ThreadLocalPointer {
- public:
- ThreadLocalPointer() : slot_() {}
-
- void Allocate() { ThreadLocalPlatform::AllocateSlot(&slot_); }
-
- void Free() { ThreadLocalPlatform::FreeSlot(slot_); }
-
- P* Get() {
- return static_cast<P*>(ThreadLocalPlatform::GetValueFromSlot(slot_));
- }
-
- void Set(P* value) { ThreadLocalPlatform::SetValueInSlot(slot_, value); }
-
- private:
- ThreadLocalPlatform::SlotType slot_;
-};
-
-} // namespace internal
-} // namespace mojo
-
-#endif // MOJO_PUBLIC_CPP_UTILITY_LIB_THREAD_LOCAL_H_
diff --git a/mojo/public/cpp/utility/lib/thread_local_posix.cc b/mojo/public/cpp/utility/lib/thread_local_posix.cc
deleted file mode 100644
index ea7343e..0000000
--- a/mojo/public/cpp/utility/lib/thread_local_posix.cc
+++ /dev/null
@@ -1,39 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "mojo/public/cpp/utility/lib/thread_local.h"
-
-#include <assert.h>
-
-namespace mojo {
-namespace internal {
-
-// static
-void ThreadLocalPlatform::AllocateSlot(SlotType* slot) {
- if (pthread_key_create(slot, nullptr) != 0) {
- assert(false);
- }
-}
-
-// static
-void ThreadLocalPlatform::FreeSlot(SlotType slot) {
- if (pthread_key_delete(slot) != 0) {
- assert(false);
- }
-}
-
-// static
-void* ThreadLocalPlatform::GetValueFromSlot(SlotType slot) {
- return pthread_getspecific(slot);
-}
-
-// static
-void ThreadLocalPlatform::SetValueInSlot(SlotType slot, void* value) {
- if (pthread_setspecific(slot, value) != 0) {
- assert(false);
- }
-}
-
-} // namespace internal
-} // namespace mojo
diff --git a/mojo/public/cpp/utility/lib/thread_local_win.cc b/mojo/public/cpp/utility/lib/thread_local_win.cc
deleted file mode 100644
index 98841f7..0000000
--- a/mojo/public/cpp/utility/lib/thread_local_win.cc
+++ /dev/null
@@ -1,39 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "mojo/public/cpp/utility/lib/thread_local.h"
-
-#include <assert.h>
-#include <windows.h>
-
-namespace mojo {
-namespace internal {
-
-// static
-void ThreadLocalPlatform::AllocateSlot(SlotType* slot) {
- *slot = TlsAlloc();
- assert(*slot != TLS_OUT_OF_INDEXES);
-}
-
-// static
-void ThreadLocalPlatform::FreeSlot(SlotType slot) {
- if (!TlsFree(slot)) {
- assert(false);
- }
-}
-
-// static
-void* ThreadLocalPlatform::GetValueFromSlot(SlotType slot) {
- return TlsGetValue(slot);
-}
-
-// static
-void ThreadLocalPlatform::SetValueInSlot(SlotType slot, void* value) {
- if (!TlsSetValue(slot, value)) {
- assert(false);
- }
-}
-
-} // namespace internal
-} // namespace mojo
diff --git a/mojo/public/cpp/utility/run_loop.h b/mojo/public/cpp/utility/run_loop.h
index 42e5950..71aa74e 100644
--- a/mojo/public/cpp/utility/run_loop.h
+++ b/mojo/public/cpp/utility/run_loop.h
@@ -25,13 +25,6 @@
RunLoop();
~RunLoop();
- // Sets up state needed for RunLoop. This must be invoked before creating a
- // RunLoop.
- static void SetUp();
-
- // Cleans state created by Setup().
- static void TearDown();
-
// Returns the RunLoop for the current thread. Returns null if not yet
// created.
static RunLoop* current();
diff --git a/mojo/public/cpp/utility/tests/run_loop_unittest.cc b/mojo/public/cpp/utility/tests/run_loop_unittest.cc
index 05d591a..fb41a41 100644
--- a/mojo/public/cpp/utility/tests/run_loop_unittest.cc
+++ b/mojo/public/cpp/utility/tests/run_loop_unittest.cc
@@ -47,25 +47,8 @@
MOJO_DISALLOW_COPY_AND_ASSIGN(TestRunLoopHandler);
};
-class RunLoopTest : public testing::Test {
- public:
- RunLoopTest() {}
-
- void SetUp() override {
- Test::SetUp();
- RunLoop::SetUp();
- }
- void TearDown() override {
- RunLoop::TearDown();
- Test::TearDown();
- }
-
- private:
- MOJO_DISALLOW_COPY_AND_ASSIGN(RunLoopTest);
-};
-
// Trivial test to verify Run() with no added handles returns.
-TEST_F(RunLoopTest, ExitsWithNoHandles) {
+TEST(RunLoopTest, ExitsWithNoHandles) {
RunLoop run_loop;
run_loop.Run();
}
@@ -90,7 +73,7 @@
};
// Verifies RunLoop quits when no more handles (handle is removed when ready).
-TEST_F(RunLoopTest, HandleReady) {
+TEST(RunLoopTest, HandleReady) {
RemoveOnReadyRunLoopHandler handler;
MessagePipe test_pipe;
EXPECT_TRUE(test::WriteTextMessage(test_pipe.handle1.get(), std::string()));
@@ -125,7 +108,7 @@
};
// Verifies Quit() from OnHandleReady() quits the loop.
-TEST_F(RunLoopTest, QuitFromReady) {
+TEST(RunLoopTest, QuitFromReady) {
QuitOnReadyRunLoopHandler handler;
MessagePipe test_pipe;
EXPECT_TRUE(test::WriteTextMessage(test_pipe.handle1.get(), std::string()));
@@ -160,7 +143,7 @@
};
// Verifies Quit() when the deadline is reached works.
-TEST_F(RunLoopTest, QuitWhenDeadlineExpired) {
+TEST(RunLoopTest, QuitWhenDeadlineExpired) {
QuitOnErrorRunLoopHandler handler;
MessagePipe test_pipe;
RunLoop run_loop;
@@ -176,7 +159,7 @@
}
// Test that handlers are notified of loop destruction.
-TEST_F(RunLoopTest, Destruction) {
+TEST(RunLoopTest, Destruction) {
TestRunLoopHandler handler;
MessagePipe test_pipe;
{
@@ -213,7 +196,7 @@
};
// Test that handlers are notified of loop destruction.
-TEST_F(RunLoopTest, MultipleHandleDestruction) {
+TEST(RunLoopTest, MultipleHandleDestruction) {
RemoveManyRunLoopHandler odd_handler;
TestRunLoopHandler even_handler;
MessagePipe test_pipe1, test_pipe2, test_pipe3;
@@ -262,7 +245,7 @@
MOJO_DISALLOW_COPY_AND_ASSIGN(AddHandlerOnErrorHandler);
};
-TEST_F(RunLoopTest, AddHandlerOnError) {
+TEST(RunLoopTest, AddHandlerOnError) {
AddHandlerOnErrorHandler handler;
MessagePipe test_pipe;
{
@@ -277,7 +260,7 @@
EXPECT_EQ(MOJO_RESULT_ABORTED, handler.last_error_result());
}
-TEST_F(RunLoopTest, Current) {
+TEST(RunLoopTest, Current) {
EXPECT_TRUE(RunLoop::current() == nullptr);
{
RunLoop run_loop;
@@ -360,7 +343,7 @@
const size_t NestingRunLoopHandler::kDepthLimit = 10;
const char NestingRunLoopHandler::kSignalMagic = 'X';
-TEST_F(RunLoopTest, NestedRun) {
+TEST(RunLoopTest, NestedRun) {
NestingRunLoopHandler handler;
MessagePipe test_pipe;
RunLoop run_loop;
@@ -388,7 +371,7 @@
std::vector<int>* sequence;
};
-TEST_F(RunLoopTest, DelayedTaskOrder) {
+TEST(RunLoopTest, DelayedTaskOrder) {
std::vector<int> sequence;
RunLoop run_loop;
run_loop.PostDelayedTask(Closure(Task(1, &sequence)), 0);
@@ -410,7 +393,7 @@
RunLoop* run_loop;
};
-TEST_F(RunLoopTest, QuitFromDelayedTask) {
+TEST(RunLoopTest, QuitFromDelayedTask) {
TestRunLoopHandler handler;
MessagePipe test_pipe;
RunLoop run_loop;
