Exempt the Service Thread from BLOCK_SHUTDOWN DCHECKs

base/task_scheduler/task_scheduler_impl_unittest.cc

 // Copyright 2016 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 "base/task_scheduler/task_scheduler_impl.h"
 
 #include <stddef.h>
 
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/synchronization/lock.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/task_scheduler/scheduler_worker_pool_params.h"
 #include "base/task_scheduler/task_traits.h"
 #include "base/task_scheduler/test_task_factory.h"
 #include "base/test/test_timeouts.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/simple_thread.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #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)
 
 namespace base {
 namespace internal {
 
 namespace {
 
 struct TraitsExecutionModePair {
@@ skipping 399 matching lines @@
               ADD_FAILURE() << "COM STA was not initialized on this thread";
               CoUninitialize();
             }
             task_ran->Signal();
           },
           com_sta_task_runner, Unretained(&task_ran)));
   task_ran.Wait();
 }
 #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