Index: base/task_scheduler/scheduler_worker_unittest.cc |
diff --git a/base/task_scheduler/scheduler_worker_unittest.cc b/base/task_scheduler/scheduler_worker_unittest.cc |
index b65d50c07c3fb5db64ba8df08031bc6934ef10a5..4682368e7fe8c86deb99b9ea30c52f1448b4e547 100644 |
--- a/base/task_scheduler/scheduler_worker_unittest.cc |
+++ b/base/task_scheduler/scheduler_worker_unittest.cc |
@@ -13,13 +13,16 @@ |
#include "base/bind_helpers.h" |
#include "base/macros.h" |
#include "base/memory/ptr_util.h" |
+#include "base/memory/ref_counted.h" |
#include "base/synchronization/condition_variable.h" |
#include "base/synchronization/waitable_event.h" |
#include "base/task_scheduler/scheduler_lock.h" |
#include "base/task_scheduler/sequence.h" |
#include "base/task_scheduler/task.h" |
#include "base/task_scheduler/task_tracker.h" |
+#include "base/test/test_timeouts.h" |
#include "base/threading/platform_thread.h" |
+#include "base/threading/simple_thread.h" |
#include "base/time/time.h" |
#include "build/build_config.h" |
#include "testing/gmock/include/gmock/gmock.h" |
@@ -121,7 +124,7 @@ class TaskSchedulerWorkerTest : public testing::TestWithParam<size_t> { |
return re_enqueued_sequences_; |
} |
- std::unique_ptr<SchedulerWorker> worker_; |
+ scoped_refptr<SchedulerWorker> worker_; |
private: |
class TestSchedulerWorkerDelegate : public SchedulerWorkerDefaultDelegate { |
@@ -355,34 +358,92 @@ namespace { |
class ControllableDetachDelegate : public SchedulerWorkerDefaultDelegate { |
public: |
- ControllableDetachDelegate(TaskTracker* task_tracker) |
- : task_tracker_(task_tracker), |
- work_processed_(WaitableEvent::ResetPolicy::MANUAL, |
- WaitableEvent::InitialState::NOT_SIGNALED), |
- detach_requested_(WaitableEvent::ResetPolicy::MANUAL, |
+ class Controls : public RefCountedThreadSafe<Controls> { |
+ public: |
+ Controls() |
+ : work_running_(WaitableEvent::ResetPolicy::MANUAL, |
+ WaitableEvent::InitialState::SIGNALED), |
+ work_processed_(WaitableEvent::ResetPolicy::MANUAL, |
WaitableEvent::InitialState::NOT_SIGNALED), |
- detached_(WaitableEvent::ResetPolicy::MANUAL, |
- WaitableEvent::InitialState::NOT_SIGNALED) { |
- EXPECT_TRUE(task_tracker_); |
- } |
+ detach_requested_(WaitableEvent::ResetPolicy::MANUAL, |
+ WaitableEvent::InitialState::NOT_SIGNALED), |
+ detached_(WaitableEvent::ResetPolicy::MANUAL, |
+ WaitableEvent::InitialState::NOT_SIGNALED), |
+ can_detach_block_(WaitableEvent::ResetPolicy::MANUAL, |
+ WaitableEvent::InitialState::SIGNALED), |
+ destroyed_(WaitableEvent::ResetPolicy::MANUAL, |
+ WaitableEvent::InitialState::NOT_SIGNALED) {} |
- ~ControllableDetachDelegate() override = default; |
+ void HaveWorkBlock() { work_running_.Reset(); } |
- // SchedulerWorker::Delegate: |
- MOCK_METHOD1(OnMainEntry, void(SchedulerWorker* worker)); |
+ void UnblockWork() { work_running_.Signal(); } |
+ |
+ void MakeCanDetachBlock() { can_detach_block_.Reset(); } |
+ |
+ void UnblockCanDetach() { can_detach_block_.Signal(); } |
+ |
+ void WaitForWorkToRun() { work_processed_.Wait(); } |
+ |
+ void WaitForDetachRequest() { detach_requested_.Wait(); } |
+ |
+ void WaitForDetach() { detached_.Wait(); } |
+ |
+ void WaitForDelegateDestroy() { destroyed_.Wait(); } |
+ |
+ void ResetState() { |
+ work_running_.Signal(); |
+ work_processed_.Reset(); |
+ detach_requested_.Reset(); |
+ can_detach_block_.Signal(); |
+ work_requested_ = false; |
+ } |
+ |
+ void set_can_detach(bool can_detach) { can_detach_ = can_detach; } |
+ |
+ private: |
+ friend class ControllableDetachDelegate; |
+ friend class RefCountedThreadSafe<Controls>; |
+ ~Controls() = default; |
+ |
+ WaitableEvent work_running_; |
+ WaitableEvent work_processed_; |
+ WaitableEvent detach_requested_; |
+ WaitableEvent detached_; |
+ WaitableEvent can_detach_block_; |
+ WaitableEvent destroyed_; |
+ |
+ bool can_detach_ = false; |
+ bool work_requested_ = false; |
+ |
+ DISALLOW_COPY_AND_ASSIGN(Controls); |
+ }; |
+ |
+ ControllableDetachDelegate(TaskTracker* task_tracker) |
+ : task_tracker_(task_tracker), controls_(new Controls()) {} |
+ |
+ ~ControllableDetachDelegate() override { controls_->destroyed_.Signal(); } |
scoped_refptr<Sequence> GetWork(SchedulerWorker* worker) |
override { |
// Sends one item of work to signal |work_processed_|. On subsequent calls, |
// sends nullptr to indicate there's no more work to be done. |
- if (work_requested_) |
+ if (controls_->work_requested_) |
return nullptr; |
- work_requested_ = true; |
+ controls_->work_requested_ = true; |
scoped_refptr<Sequence> sequence(new Sequence); |
std::unique_ptr<Task> task(new Task( |
- FROM_HERE, Bind(&WaitableEvent::Signal, Unretained(&work_processed_)), |
- TaskTraits(), TimeDelta())); |
+ FROM_HERE, |
+ Bind( |
+ [](WaitableEvent* work_processed, WaitableEvent* work_running) { |
+ work_processed->Signal(); |
+ work_running->Wait(); |
+ }, |
+ Unretained(&controls_->work_processed_), |
+ Unretained(&controls_->work_running_)), |
+ TaskTraits().WithBaseSyncPrimitives().WithShutdownBehavior( |
+ TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN), |
+ TimeDelta())); |
EXPECT_TRUE(task_tracker_->WillPostTask(task.get())); |
sequence->PushTask(std::move(task)); |
return sequence; |
@@ -391,43 +452,42 @@ class ControllableDetachDelegate : public SchedulerWorkerDefaultDelegate { |
void DidRunTask() override {} |
bool CanDetach(SchedulerWorker* worker) override { |
- detach_requested_.Signal(); |
- return can_detach_; |
+ // Saving |can_detach_| now so that callers waiting on |detach_requested_| |
+ // have the thread go to sleep and then allow detachment. |
+ bool can_detach = controls_->can_detach_; |
+ controls_->detach_requested_.Signal(); |
+ controls_->can_detach_block_.Wait(); |
+ return can_detach; |
} |
void OnDetach() override { |
- EXPECT_TRUE(can_detach_); |
- EXPECT_TRUE(detach_requested_.IsSignaled()); |
- detached_.Signal(); |
+ EXPECT_TRUE(controls_->can_detach_); |
+ EXPECT_TRUE(controls_->detach_requested_.IsSignaled()); |
+ controls_->detached_.Signal(); |
} |
- void WaitForWorkToRun() { |
- work_processed_.Wait(); |
- } |
+ // ControllableDetachDelegate: |
gab
2017/02/21 19:01:04
This usually means that it implements that interfa
robliao
2017/02/21 22:26:57
This was habit for me working in C++ interfaces fo
|
+ scoped_refptr<Controls> controls() { return controls_; } |
gab
2017/02/21 19:01:04
Does this need to expose the ref (usage seems to n
robliao
2017/02/21 22:26:57
This needs to be owned for cases where we race bet
|
- void WaitForDetachRequest() { |
- detach_requested_.Wait(); |
- } |
+ private: |
+ scoped_refptr<Sequence> work_sequence_; |
+ TaskTracker* const task_tracker_; |
+ scoped_refptr<Controls> controls_; |
- void WaitForDetach() { detached_.Wait(); } |
+ DISALLOW_COPY_AND_ASSIGN(ControllableDetachDelegate); |
+}; |
- void ResetState() { |
- work_requested_ = false; |
- work_processed_.Reset(); |
- detach_requested_.Reset(); |
- } |
+class MockedControllableDetachDelegate : public ControllableDetachDelegate { |
+ public: |
+ MockedControllableDetachDelegate(TaskTracker* task_tracker) |
+ : ControllableDetachDelegate(task_tracker){}; |
+ ~MockedControllableDetachDelegate() = default; |
- void set_can_detach(bool can_detach) { can_detach_ = can_detach; } |
+ // SchedulerWorker::Delegate: |
+ MOCK_METHOD1(OnMainEntry, void(SchedulerWorker* worker)); |
private: |
- TaskTracker* const task_tracker_; |
- bool work_requested_ = false; |
- bool can_detach_ = false; |
- WaitableEvent work_processed_; |
- WaitableEvent detach_requested_; |
- WaitableEvent detached_; |
- |
- DISALLOW_COPY_AND_ASSIGN(ControllableDetachDelegate); |
+ DISALLOW_COPY_AND_ASSIGN(MockedControllableDetachDelegate); |
}; |
} // namespace |
@@ -435,50 +495,224 @@ class ControllableDetachDelegate : public SchedulerWorkerDefaultDelegate { |
TEST(TaskSchedulerWorkerTest, WorkerDetaches) { |
TaskTracker task_tracker; |
// Will be owned by SchedulerWorker. |
- ControllableDetachDelegate* delegate = |
- new StrictMock<ControllableDetachDelegate>(&task_tracker); |
- delegate->set_can_detach(true); |
+ MockedControllableDetachDelegate* delegate = |
+ new StrictMock<MockedControllableDetachDelegate>(&task_tracker); |
+ scoped_refptr<ControllableDetachDelegate::Controls> controls = |
+ delegate->controls(); |
+ controls->set_can_detach(true); |
EXPECT_CALL(*delegate, OnMainEntry(_)); |
- std::unique_ptr<SchedulerWorker> worker = |
- SchedulerWorker::Create( |
- ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker, |
- SchedulerWorker::InitialState::ALIVE); |
+ scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create( |
+ ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker, |
+ SchedulerWorker::InitialState::ALIVE); |
worker->WakeUp(); |
- delegate->WaitForWorkToRun(); |
+ controls->WaitForWorkToRun(); |
Mock::VerifyAndClear(delegate); |
- delegate->WaitForDetachRequest(); |
- delegate->WaitForDetach(); |
+ controls->WaitForDetachRequest(); |
+ controls->WaitForDetach(); |
ASSERT_FALSE(worker->ThreadAliveForTesting()); |
} |
+TEST(TaskSchedulerWorkerTest, WorkerReleasesBeforeDetach) { |
+ TaskTracker task_tracker; |
+ // Will be owned by SchedulerWorker. |
+ // No mock here as that's reasonably covered by other tests and the delegate |
+ // may destroy on a different thread. Mocks aren't designed with that in mind. |
+ std::unique_ptr<ControllableDetachDelegate> delegate = |
+ MakeUnique<ControllableDetachDelegate>(&task_tracker); |
+ scoped_refptr<ControllableDetachDelegate::Controls> controls = |
+ delegate->controls(); |
+ |
+ controls->set_can_detach(true); |
+ controls->MakeCanDetachBlock(); |
+ |
+ scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create( |
+ ThreadPriority::NORMAL, std::move(delegate), &task_tracker, |
+ SchedulerWorker::InitialState::ALIVE); |
+ worker->WakeUp(); |
+ |
+ controls->WaitForDetachRequest(); |
+ worker->Cleanup(); |
+ worker = nullptr; |
+ controls->UnblockCanDetach(); |
+ controls->WaitForDelegateDestroy(); |
+} |
+ |
+TEST(TaskSchedulerWorkerTest, WorkerDetachesAndSelfDestroysAfterDetach) { |
+ TaskTracker task_tracker; |
+ // Will be owned by SchedulerWorker. |
+ // No mock here as that's reasonably covered by other tests and the delegate |
+ // may destroy on a different thread. Mocks aren't designed with that in mind. |
+ std::unique_ptr<ControllableDetachDelegate> delegate = |
+ MakeUnique<ControllableDetachDelegate>(&task_tracker); |
+ scoped_refptr<ControllableDetachDelegate::Controls> controls = |
+ delegate->controls(); |
+ |
+ controls->set_can_detach(true); |
+ |
+ scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create( |
+ ThreadPriority::NORMAL, std::move(delegate), &task_tracker, |
+ SchedulerWorker::InitialState::ALIVE); |
+ worker->WakeUp(); |
+ |
+ controls->WaitForDetach(); |
+ worker->Cleanup(); |
+ worker = nullptr; |
+ controls->WaitForDelegateDestroy(); |
+} |
+ |
+TEST(TaskSchedulerWorkerTest, WorkerDetachesAndSelfDestroysDuringWork) { |
+ TaskTracker task_tracker; |
+ // Will be owned by SchedulerWorker. |
+ // No mock here as that's reasonably covered by other tests and the delegate |
+ // may destroy on a different thread. Mocks aren't designed with that in mind. |
+ std::unique_ptr<ControllableDetachDelegate> delegate = |
+ MakeUnique<ControllableDetachDelegate>(&task_tracker); |
+ scoped_refptr<ControllableDetachDelegate::Controls> controls = |
+ delegate->controls(); |
+ |
+ controls->set_can_detach(true); |
+ controls->HaveWorkBlock(); |
+ |
+ scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create( |
+ ThreadPriority::NORMAL, std::move(delegate), &task_tracker, |
+ SchedulerWorker::InitialState::ALIVE); |
+ worker->WakeUp(); |
+ |
+ controls->WaitForWorkToRun(); |
+ worker->Cleanup(); |
+ worker = nullptr; |
+ controls->UnblockWork(); |
+ controls->WaitForDelegateDestroy(); |
+} |
+ |
+TEST(TaskSchedulerWorkerTest, WorkerDetachesAndSelfDestroysDuringWait) { |
+ TaskTracker task_tracker; |
+ // Will be owned by SchedulerWorker. |
+ // No mock here as that's reasonably covered by other tests and the delegate |
+ // may destroy on a different thread. Mocks aren't designed with that in mind. |
+ std::unique_ptr<ControllableDetachDelegate> delegate = |
+ MakeUnique<ControllableDetachDelegate>(&task_tracker); |
+ scoped_refptr<ControllableDetachDelegate::Controls> controls = |
+ delegate->controls(); |
+ |
+ scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create( |
+ ThreadPriority::NORMAL, std::move(delegate), &task_tracker, |
+ SchedulerWorker::InitialState::ALIVE); |
+ worker->WakeUp(); |
+ |
+ controls->WaitForDetachRequest(); |
+ controls->set_can_detach(true); |
gab
2017/02/21 19:01:04
What's the point of setting it after? (I guess I a
robliao
2017/02/21 22:26:57
In this case, we just want to wait for a CanDetach
|
+ worker->Cleanup(); |
+ worker = nullptr; |
+ controls->WaitForDelegateDestroy(); |
+} |
+ |
+TEST(TaskSchedulerWorkerTest, WorkerDetachesAndSelfDestroysDuringShutdown) { |
+ TaskTracker task_tracker; |
+ // Will be owned by SchedulerWorker. |
+ // No mock here as that's reasonably covered by other tests and the delegate |
+ // may destroy on a different thread. Mocks aren't designed with that in mind. |
+ std::unique_ptr<ControllableDetachDelegate> delegate = |
+ MakeUnique<ControllableDetachDelegate>(&task_tracker); |
+ scoped_refptr<ControllableDetachDelegate::Controls> controls = |
+ delegate->controls(); |
+ |
+ controls->HaveWorkBlock(); |
+ |
+ scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create( |
+ ThreadPriority::NORMAL, std::move(delegate), &task_tracker, |
+ SchedulerWorker::InitialState::ALIVE); |
+ worker->WakeUp(); |
+ |
+ controls->WaitForWorkToRun(); |
+ task_tracker.Shutdown(); |
+ worker->Cleanup(); |
+ worker = nullptr; |
+ controls->UnblockWork(); |
+ controls->WaitForDelegateDestroy(); |
+} |
+ |
+namespace { |
+ |
+class CallJoinFromDifferentThread : public SimpleThread { |
+ public: |
+ CallJoinFromDifferentThread(SchedulerWorker* worker_to_join) |
+ : SimpleThread("SchedulerWorkerJoinThread"), |
+ worker_to_join_(worker_to_join) {} |
+ |
+ ~CallJoinFromDifferentThread() override = default; |
+ |
+ void Run() override { worker_to_join_->JoinForTesting(); } |
+ |
+ private: |
+ SchedulerWorker* const worker_to_join_; |
+ DISALLOW_COPY_AND_ASSIGN(CallJoinFromDifferentThread); |
+}; |
+ |
+} // namespace |
+ |
+TEST(TaskSchedulerWorkerTest, WorkerDetachesAndSelfDestroysDuringJoin) { |
+ TaskTracker task_tracker; |
+ // Will be owned by SchedulerWorker. |
+ // No mock here as that's reasonably covered by other tests and the |
+ // delegate may destroy on a different thread. Mocks aren't designed with that |
+ // in mind. |
+ std::unique_ptr<ControllableDetachDelegate> delegate = |
+ MakeUnique<ControllableDetachDelegate>(&task_tracker); |
+ scoped_refptr<ControllableDetachDelegate::Controls> controls = |
+ delegate->controls(); |
+ |
+ controls->HaveWorkBlock(); |
+ |
+ scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create( |
+ ThreadPriority::NORMAL, std::move(delegate), &task_tracker, |
+ SchedulerWorker::InitialState::ALIVE); |
+ worker->WakeUp(); |
+ |
+ controls->WaitForWorkToRun(); |
+ CallJoinFromDifferentThread join_from_different_thread(worker.get()); |
+ join_from_different_thread.Start(); |
+ // While Start blocks until the thread starts, that doesn't cover the actual |
gab
2017/02/21 19:01:04
Start no longer blocks until thread starts (Franco
robliao
2017/02/21 22:26:57
SimpleThread::Start() waits.
Thread::Start() does
gab
2017/02/22 18:02:24
Ah, found it: https://codereview.chromium.org/2664
robliao
2017/02/22 20:43:39
Went with the event. Amended comment.
|
+ // join call. Yield here to give the other thread a chance to call |
+ // JoinForTesting(). |
+ PlatformThread::YieldCurrentThread(); |
+ worker->Cleanup(); |
+ worker = nullptr; |
+ controls->UnblockWork(); |
+ controls->WaitForDelegateDestroy(); |
+ join_from_different_thread.Join(); |
+} |
+ |
TEST(TaskSchedulerWorkerTest, WorkerDetachesAndWakes) { |
TaskTracker task_tracker; |
// Will be owned by SchedulerWorker. |
- ControllableDetachDelegate* delegate = |
- new StrictMock<ControllableDetachDelegate>(&task_tracker); |
- delegate->set_can_detach(true); |
+ MockedControllableDetachDelegate* delegate = |
+ new StrictMock<MockedControllableDetachDelegate>(&task_tracker); |
+ scoped_refptr<ControllableDetachDelegate::Controls> controls = |
+ delegate->controls(); |
+ |
+ controls->set_can_detach(true); |
EXPECT_CALL(*delegate, OnMainEntry(_)); |
- std::unique_ptr<SchedulerWorker> worker = |
- SchedulerWorker::Create( |
- ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker, |
- SchedulerWorker::InitialState::ALIVE); |
+ scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create( |
+ ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker, |
+ SchedulerWorker::InitialState::ALIVE); |
worker->WakeUp(); |
- delegate->WaitForWorkToRun(); |
+ controls->WaitForWorkToRun(); |
Mock::VerifyAndClear(delegate); |
- delegate->WaitForDetachRequest(); |
- delegate->WaitForDetach(); |
+ controls->WaitForDetachRequest(); |
+ controls->WaitForDetach(); |
ASSERT_FALSE(worker->ThreadAliveForTesting()); |
- delegate->ResetState(); |
- delegate->set_can_detach(false); |
+ controls->ResetState(); |
+ controls->set_can_detach(false); |
// Expect OnMainEntry() to be called when SchedulerWorker recreates its |
// thread. |
EXPECT_CALL(*delegate, OnMainEntry(worker.get())); |
worker->WakeUp(); |
- delegate->WaitForWorkToRun(); |
+ controls->WaitForWorkToRun(); |
Mock::VerifyAndClear(delegate); |
- delegate->WaitForDetachRequest(); |
- delegate->WaitForDetach(); |
+ controls->WaitForDetachRequest(); |
+ controls->WaitForDetach(); |
ASSERT_TRUE(worker->ThreadAliveForTesting()); |
worker->JoinForTesting(); |
} |
@@ -486,18 +720,19 @@ TEST(TaskSchedulerWorkerTest, WorkerDetachesAndWakes) { |
TEST(TaskSchedulerWorkerTest, CreateDetached) { |
TaskTracker task_tracker; |
// Will be owned by SchedulerWorker. |
- ControllableDetachDelegate* delegate = |
- new StrictMock<ControllableDetachDelegate>(&task_tracker); |
- std::unique_ptr<SchedulerWorker> worker = |
- SchedulerWorker::Create( |
- ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker, |
- SchedulerWorker::InitialState::DETACHED); |
+ MockedControllableDetachDelegate* delegate = |
+ new StrictMock<MockedControllableDetachDelegate>(&task_tracker); |
+ scoped_refptr<ControllableDetachDelegate::Controls> controls = |
+ delegate->controls(); |
+ scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create( |
+ ThreadPriority::NORMAL, WrapUnique(delegate), &task_tracker, |
+ SchedulerWorker::InitialState::DETACHED); |
ASSERT_FALSE(worker->ThreadAliveForTesting()); |
EXPECT_CALL(*delegate, OnMainEntry(worker.get())); |
worker->WakeUp(); |
- delegate->WaitForWorkToRun(); |
+ controls->WaitForWorkToRun(); |
Mock::VerifyAndClear(delegate); |
- delegate->WaitForDetachRequest(); |
+ controls->WaitForDetachRequest(); |
ASSERT_TRUE(worker->ThreadAliveForTesting()); |
worker->JoinForTesting(); |
} |
@@ -560,7 +795,7 @@ TEST(TaskSchedulerWorkerTest, BumpPriorityOfAliveThreadDuringShutdown) { |
? ThreadPriority::BACKGROUND |
: ThreadPriority::NORMAL); |
- std::unique_ptr<SchedulerWorker> worker = SchedulerWorker::Create( |
+ scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create( |
ThreadPriority::BACKGROUND, std::move(delegate), &task_tracker, |
SchedulerWorker::InitialState::ALIVE); |
@@ -587,7 +822,7 @@ TEST(TaskSchedulerWorkerTest, BumpPriorityOfDetachedThreadDuringShutdown) { |
delegate_raw->SetExpectedThreadPriority(ThreadPriority::NORMAL); |
// Create a DETACHED thread. |
- std::unique_ptr<SchedulerWorker> worker = SchedulerWorker::Create( |
+ scoped_refptr<SchedulerWorker> worker = SchedulerWorker::Create( |
ThreadPriority::BACKGROUND, std::move(delegate), &task_tracker, |
SchedulerWorker::InitialState::DETACHED); |