[Sync] Signal UIModelWorker to abort on sync shutdown.

components/sync/driver/glue/ui_model_worker.cc

Index: components/sync/driver/glue/ui_model_worker.cc
diff --git a/components/sync/driver/glue/ui_model_worker.cc b/components/sync/driver/glue/ui_model_worker.cc
index c812f9a9b54ed6b95cf0c974a3d07f778f2eac48..592b1ee00dd76e0b7bc106361857ff4397631e1d 100644
--- a/components/sync/driver/glue/ui_model_worker.cc
+++ b/components/sync/driver/glue/ui_model_worker.cc
@@ -9,7 +9,6 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
-#include "base/synchronization/waitable_event.h"
 #include "components/sync/base/scoped_event_signal.h"
 
 namespace syncer {
@@ -19,6 +18,11 @@ namespace {
 void CallDoWorkAndSignalEvent(const WorkCallback& work,
                               syncer::ScopedEventSignal scoped_event_signal,
                               SyncerError* error_info) {
+  if (scoped_event_signal.IsSignaled()) {
+    *error_info = CANNOT_DO_WORK;
+    return;
+  }
+
   *error_info = work.Run();
   // The event in |scoped_event_signal| is signaled at the end of this scope.
 }
@@ -27,7 +31,10 @@ void CallDoWorkAndSignalEvent(const WorkCallback& work,
 
 UIModelWorker::UIModelWorker(
     scoped_refptr<base::SingleThreadTaskRunner> ui_thread)
-    : ui_thread_(std::move(ui_thread)) {}
+    : ui_thread_(std::move(ui_thread)),
+      work_done_or_abandoned_(base::WaitableEvent::ResetPolicy::MANUAL,
+                              base::WaitableEvent::InitialState::NOT_SIGNALED) {
+}
 
 SyncerError UIModelWorker::DoWorkAndWaitUntilDoneImpl(
     const WorkCallback& work) {
@@ -38,26 +45,29 @@ SyncerError UIModelWorker::DoWorkAndWaitUntilDoneImpl(
     return work.Run();
   }
 
-  // Signaled when the task is deleted, i.e. after it runs or when it is
-  // abandoned.
-  base::WaitableEvent work_done_or_abandoned(
-      base::WaitableEvent::ResetPolicy::AUTOMATIC,
-      base::WaitableEvent::InitialState::NOT_SIGNALED);
+  // RequestStop hasn't been called so reset the signal to do more work.
+  work_done_or_abandoned_.Reset();

[fdoray, 2016/11/14 14:24:34]

This can happen:
THREAD  ACTION
Sync:   DoWorkAndWaitUntilDone()
UI:     UIModelWorker::RequestStop()
Sync:   DoWorkAndWaitUntilDoneImpl()
Sync:   work_done_or_abandoned_.Reset() !!!
Sync:   ui_thread_->PostTask(...)
Sync:   work_done_or_abandoned_.Wait() !!! Blocked forever

For that reason, I would use 2 events:

SyncerError UIModelWorker::DoWorkAndWaitUntilDoneImpl(...) {
  ...

  // Signaled when the task is deleted, i.e. after it runs or when it is
  // abandoned.
  base::WaitableEvent work_done_or_abandoned(
      base::WaitableEvent::ResetPolicy::AUTOMATIC,
      base::WaitableEvent::InitialState::NOT_SIGNALED);
  
  if (!ui_thread_->PostTask(...)) {
     ...
  }
  
  base::WaitableEvent* events[] = {&work_done_or_abandoned, &stop_requested_};
  base::WaitableEvent::WaitMany(events, arraysize(events));

  ...
}

void UIModelWorker::RequestStop() {
  DCHECK(ui_thread_->BelongsToCurrentThread());
  ModelSafeWorker::RequestStop();
  stop_requested_.Signal();
}
  
WDYT?

[maxbogue, 2016/11/14 20:53:21]

This actually occurred to me on Friday right before I stopped working... I'm
just going to pass worker as a parameter and check IsStopped before running the
work, since that's more "the right thing to do". I didn't know about WaitMany
though, that's neat.

 
   if (!ui_thread_->PostTask(FROM_HERE,
                             base::Bind(&CallDoWorkAndSignalEvent, work,
                                        base::Passed(syncer::ScopedEventSignal(
-                                           &work_done_or_abandoned)),
+                                           &work_done_or_abandoned_)),
                                        &error_info))) {
     DLOG(WARNING) << "Could not post work to UI loop.";
     error_info = CANNOT_DO_WORK;
     return error_info;
   }
-  work_done_or_abandoned.Wait();
+  work_done_or_abandoned_.Wait();
 
   return error_info;
 }
 
+void UIModelWorker::RequestStop() {
+  DCHECK(ui_thread_->BelongsToCurrentThread());
+  ModelSafeWorker::RequestStop();
+  work_done_or_abandoned_.Signal();
+}
+
 ModelSafeGroup UIModelWorker::GetModelSafeGroup() {
   return GROUP_UI;
 }