Exempt the Service Thread from BLOCK_SHUTDOWN DCHECKs

base/task_scheduler/task_scheduler_impl_unittest.cc

Index: base/task_scheduler/task_scheduler_impl_unittest.cc
diff --git a/base/task_scheduler/task_scheduler_impl_unittest.cc b/base/task_scheduler/task_scheduler_impl_unittest.cc
index b8d8fad8343808296bff8dd7acedba946bdf8e4e..b7e7c2f7a9654785351b5a565c0f21326cba1c9f 100644
--- a/base/task_scheduler/task_scheduler_impl_unittest.cc
+++ b/base/task_scheduler/task_scheduler_impl_unittest.cc
@@ -28,6 +28,15 @@
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"

[fdoray, 2017/05/03 20:34:33]

#include "build/build_config.h"

[robliao, 2017/05/03 21:11:17]

Done.

 
+#if defined(OS_POSIX)
+#include <unistd.h>
+
+#include "base/debug/leak_annotations.h"
+#include "base/files/file_descriptor_watcher_posix.h"
+#include "base/files/file_util.h"
+#include "base/posix/eintr_wrapper.h"
+#endif  // defined(OS_POSIX)
+
 #if defined(OS_WIN)
 #include <objbase.h>
 #endif  // defined(OS_WIN)
@@ -447,5 +456,57 @@ TEST_F(TaskSchedulerImplTest, COMSTATaskRunnersRunWithCOMSTA) {
 }
 #endif  // defined(OS_WIN)
 
+TEST_F(TaskSchedulerImplTest, DelayedTasksNotRunAfterShutdown) {
+  StartTaskScheduler();
+  scheduler_.PostDelayedTaskWithTraits(FROM_HERE, TaskTraits(),

[fdoray, 2017/05/03 20:34:33]

Comment that this is racy or make it non-racy.

Post task A to sequence 1. This task waits on a barrier WaitableEvent.
Post delayed task B to sequence 1. This task has an ADD_FAILURE().
Shutdown. It is guaranteed that task B has not run yet since it is sequenced
with a blocked task.
Signal barrier.
Wait a little bit and make sure that task B does not run. 

[robliao, 2017/05/03 21:11:17]

Seems like the barrier just ensures that a task actually ran, but it doesn't
change the race between the timeout and the delayed task. Added a comment.

[fdoray, 2017/05/04 18:37:32]

In the current state, the test may fail if the 50 ms timeout expires before
Shutdown() is called. With the change I proposed, this would not be possible.

The race between the timeout and the delayed task still exists (the delayed task
was gonna run but didn't because the 100 ms delay wasn't long enough), but at
least it doesn't lead to a test failure.

[robliao, 2017/05/04 19:01:48]

Discussed offline. Shutdown() would block, which means we would have to release
the WaitableEvent on a different thread. Added a comment documenting the
decision.

+                                       BindOnce([]() { ADD_FAILURE(); }),
+                                       TimeDelta::FromMilliseconds(50));

[fdoray, 2017/05/03 20:34:33]

TestTimeouts::tiny_timeout() here and 2*TestTimeouts::tiny_timeout() below

[robliao, 2017/05/03 21:11:17]

Done.

+  scheduler_.Shutdown();
+  PlatformThread::Sleep(TimeDelta::FromMilliseconds(100));
+}
+
+#if defined(OS_POSIX)
+
+TEST_F(TaskSchedulerImplTest, FileDescriptorWatcherNoOpsAfterShutdown) {
+  StartTaskScheduler();
+
+  int pipes[2];
+  ASSERT_EQ(0, pipe(pipes));
+
+  scoped_refptr<TaskRunner> blocking_task_runner =
+      scheduler_.CreateSequencedTaskRunnerWithTraits(
+          TaskTraits().WithShutdownBehavior(
+              TaskShutdownBehavior::BLOCK_SHUTDOWN));
+  blocking_task_runner->PostTask(
+      FROM_HERE,
+      BindOnce(
+          [](int read_fd) {
+            std::unique_ptr<FileDescriptorWatcher::Controller> controller =
+                FileDescriptorWatcher::WatchReadable(
+                    read_fd, BindRepeating([]() { NOTREACHED(); }));
+
+            // This test is for components that intentionally leak their
+            // watchers at shutdown. We can't clean |controller| up because its
+            // destructor will assert that it's being called from the correct
+            // sequence. After the task scheduler is shutdown, it is not
+            // possible to run tasks on this sequence.
+            ANNOTATE_LEAKING_OBJECT_PTR(controller.get());
+            controller.release();
+          },
+          pipes[0]));
+
+  scheduler_.Shutdown();
+
+  constexpr char kByte = '!';
+  ASSERT_TRUE(WriteFileDescriptor(pipes[1], &kByte, sizeof(kByte)));
+
+  // Give a chance for the file watcher to fire before closing the handles.
+  PlatformThread::Sleep(TimeDelta::FromMilliseconds(100));
+
+  EXPECT_EQ(0, IGNORE_EINTR(close(pipes[0])));
+  EXPECT_EQ(0, IGNORE_EINTR(close(pipes[1])));
+}
+#endif  // defined(OS_POSIX)
+
 }  // namespace internal
 }  // namespace base