Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(246)

Unified Diff: base/threading/thread_unittest.cc

Issue 2135413003: Add |joinable| to Thread::Options (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Init() => InstallMessageLoop() -- Init was already used on Thread::... Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: base/threading/thread_unittest.cc
diff --git a/base/threading/thread_unittest.cc b/base/threading/thread_unittest.cc
index dfa04c6ac48fce2e445daccb4e14911bda951757..52ba3a891a77a8170a45a0c3821f2874aa34acf0 100644
--- a/base/threading/thread_unittest.cc
+++ b/base/threading/thread_unittest.cc
@@ -9,14 +9,26 @@
#include <vector>
#include "base/bind.h"
-#include "base/location.h"
+#include "base/macros.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/synchronization/waitable_event.h"
#include "base/third_party/dynamic_annotations/dynamic_annotations.h"
+#include "base/threading/platform_thread.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/platform_test.h"
+// Death tests misbehave on Android.
+// TODO(gab): Remove this when https://codereview.chromium.org/2162053006
+// lands.
+#if DCHECK_IS_ON() && defined(GTEST_HAS_DEATH_TEST) && !defined(OS_ANDROID)
+#define EXPECT_DCHECK_DEATH(statement, regex) EXPECT_DEATH(statement, regex)
+#else
+#define EXPECT_DCHECK_DEATH(statement, regex)
+#endif
+
using base::Thread;
typedef PlatformTest ThreadTest;
@@ -43,8 +55,11 @@ class SleepInsideInitThread : public Thread {
init_called_ = true;
}
bool InitCalled() { return init_called_; }
+
private:
bool init_called_;
+
+ DISALLOW_COPY_AND_ASSIGN(SleepInsideInitThread);
};
enum ThreadEvent {
@@ -81,6 +96,8 @@ class CaptureToEventList : public Thread {
private:
EventList* event_list_;
+
+ DISALLOW_COPY_AND_ASSIGN(CaptureToEventList);
};
// Observer that writes a value into |event_list| when a message loop has been
@@ -101,6 +118,8 @@ class CapturingDestructionObserver
private:
EventList* event_list_;
+
+ DISALLOW_COPY_AND_ASSIGN(CapturingDestructionObserver);
};
// Task that adds a destruction observer to the current message loop.
@@ -164,10 +183,47 @@ TEST_F(ThreadTest, StartWithOptions_StackSize) {
event.Wait();
}
-TEST_F(ThreadTest, TwoTasks) {
+TEST_F(ThreadTest, StartWithOptions_NonJoinable) {
+ Thread a("StartNonJoinable");
+ Thread::Options options;
+ options.joinable = false;
+ EXPECT_TRUE(a.StartWithOptions(options));
+ EXPECT_TRUE(a.message_loop());
+ EXPECT_TRUE(a.IsRunning());
+
+ // Without this call this test is racy. The above IsRunning() succeeds because
+ // of an early-return condition while between Start() and Stop(), after
+ // invoking Stop() below this early-return condition is no longer satisfied
+ // and the real |is_running_| bit has to be checked. It could still be false
+ // if the message loop hasn't started for real in practice. This is only a
+ // requirement for this test because the non-joinable thread makes it possible
+ // to return from Stop() before the thread has even started.
+ EXPECT_TRUE(a.WaitUntilThreadStarted());
+
+ // Make the thread block until |block_event| is signaled.
+ base::WaitableEvent block_event(
+ base::WaitableEvent::ResetPolicy::AUTOMATIC,
+ base::WaitableEvent::InitialState::NOT_SIGNALED);
+ a.task_runner()->PostTask(
+ FROM_HERE,
+ base::Bind(&base::WaitableEvent::Wait, base::Unretained(&block_event)));
+
+ // Stop() shouldn't block despite the thread still being alive.
+ a.Stop();
+ EXPECT_TRUE(a.IsRunning());
+
+ // Unblock the task and give a bit of extra time to unwind QuitWhenIdle().
+ block_event.Signal();
+ base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(20));
+
+ // The thread should now have stopped on its own.
+ EXPECT_FALSE(a.IsRunning());
+}
+
+TEST_F(ThreadTest, TwoTasksOnJoinableThread) {
bool was_invoked = false;
{
- Thread a("TwoTasks");
+ Thread a("TwoTasksOnJoinableThread");
EXPECT_TRUE(a.Start());
EXPECT_TRUE(a.message_loop());
@@ -231,6 +287,39 @@ TEST_F(ThreadTest, StartTwice) {
EXPECT_FALSE(a.IsRunning());
}
+TEST_F(ThreadTest, StartTwiceNonJoinableNotAllowed) {
+ Thread a("StartTwiceNonJoinable");
+
+ Thread::Options options;
+ options.joinable = false;
+ EXPECT_TRUE(a.StartWithOptions(options));
+ EXPECT_TRUE(a.message_loop());
+ EXPECT_TRUE(a.IsRunning());
+
+ // Signaled when last task on |a| is processed.
+ base::WaitableEvent last_task_event(
+ base::WaitableEvent::ResetPolicy::AUTOMATIC,
+ base::WaitableEvent::InitialState::NOT_SIGNALED);
+ a.task_runner()->PostTask(FROM_HERE,
+ base::Bind(&base::WaitableEvent::Signal,
+ base::Unretained(&last_task_event)));
+
+ // Stop() is non-blocking, but Yield() to |a|, wait for last task to be
+ // processed and a little more for QuitWhenIdle() to unwind before considering
+ // the thread "stopped".
+ a.Stop();
+ base::PlatformThread::YieldCurrentThread();
+ last_task_event.Wait();
+ base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(20));
+
+ // This test assumes that the above was sufficient to let the thread fully
+ // stop.
+ ASSERT_FALSE(a.IsRunning());
+
+ // Restarting it should not be allowed.
+ EXPECT_DCHECK_DEATH(a.Start(), "");
+}
+
TEST_F(ThreadTest, ThreadName) {
Thread a("ThreadName");
EXPECT_TRUE(a.Start());
@@ -332,3 +421,49 @@ TEST_F(ThreadTest, MultipleWaitUntilThreadStarted) {
EXPECT_TRUE(a.WaitUntilThreadStarted());
EXPECT_TRUE(a.WaitUntilThreadStarted());
}
+
+namespace {
+
+// A Thread which uses a MessageLoop on the stack. It won't start a real
+// underlying thread (instead its messages can be processed by a RunLoop on the
+// stack).
+class ExternalMessageLoopThread : public Thread {
+ public:
+ ExternalMessageLoopThread() : Thread("ExternalMessageLoopThread") {}
+
+ ~ExternalMessageLoopThread() override { Stop(); }
+
+ void InstallMessageLoop() { SetMessageLoop(&external_message_loop_); }
+
+ private:
+ base::MessageLoop external_message_loop_;
+
+ DISALLOW_COPY_AND_ASSIGN(ExternalMessageLoopThread);
+};
+
+} // namespace
+
+TEST_F(ThreadTest, ExternalMessageLoop) {
+ ExternalMessageLoopThread a;
+ EXPECT_FALSE(a.message_loop());
+ EXPECT_FALSE(a.IsRunning());
+
+ a.InstallMessageLoop();
+ EXPECT_TRUE(a.message_loop());
+ EXPECT_TRUE(a.IsRunning());
+
+ bool ran = false;
+ a.task_runner()->PostTask(
+ FROM_HERE, base::Bind([](bool* toggled) { *toggled = true; }, &ran));
+ base::RunLoop().RunUntilIdle();
+ EXPECT_TRUE(ran);
+
+ a.Stop();
+ EXPECT_FALSE(a.message_loop());
+ EXPECT_FALSE(a.IsRunning());
+
+ // Confirm that running any remaining tasks posted from Stop() goes smoothly
+ // (e.g. https://codereview.chromium.org/2135413003/#ps300001 crashed if
+ // StopSoon() posted Thread::ThreadQuitHelper() while |run_loop_| was null).
+ base::RunLoop().RunUntilIdle();
+}

Powered by Google App Engine
This is Rietveld 408576698