Use a ConditionVariable in place of WaitableEvent in BookmarkModelWorker....

chrome/browser/sync/glue/bookmark_model_worker.h

 // Copyright (c) 2006-2008 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.
 
 #ifndef CHROME_BROWSER_SYNC_GLUE_BOOKMARK_MODEL_WORKER_H_
 #define CHROME_BROWSER_SYNC_GLUE_BOOKMARK_MODEL_WORKER_H_
 
+#include "base/condition_variable.h"
 #include "base/lock.h"
 #include "base/task.h"
 #include "base/waitable_event.h"
 #include "chrome/browser/sync/engine/syncapi.h"
 
 class MessageLoop;
 
 namespace browser_sync {
 
 // A ModelSafeWorker for bookmarks that accepts work requests from the syncapi
 // that need to be fulfilled from the MessageLoop home to the BookmarkModel
 // (this is typically the "main" UI thread).
 //
 // Lifetime note: Instances of this class will generally be owned by the
 // SyncerThread. When the SyncerThread _object_ is destroyed, the
 // BookmarkModelWorker will be destroyed. The SyncerThread object is destroyed
 // after the actual syncer pthread has exited.
 class BookmarkModelWorker
     : public sync_api::ModelSafeWorkerInterface {
  public:
   explicit BookmarkModelWorker(MessageLoop* bookmark_model_loop)
       : state_(WORKING),
         pending_work_(NULL),
         syncapi_has_shutdown_(false),
         bookmark_model_loop_(bookmark_model_loop),
-        syncapi_event_(false, false) {
+        syncapi_event_(&lock_) {
   }
   virtual ~BookmarkModelWorker();
 
   // A simple task to signal a waitable event after calling DoWork on a visitor.
   class CallDoWorkAndSignalTask : public Task {
    public:
     CallDoWorkAndSignalTask(ModelSafeWorkerInterface::Visitor* visitor,
                             base::WaitableEvent* work_done,
                             BookmarkModelWorker* scheduler)
         : visitor_(visitor), work_done_(work_done), scheduler_(scheduler) {
@@ skipping 54 matching lines @@
 
   // This is set by the UI thread, but is not explicitly thread safe, so only
   // read this value from other threads when you know it is absolutely safe (e.g
   // there is _no_ way we can be in CallDoWork with state_ = STOPPED, so it is
   // safe to read / compare in this case).
   State state_;
 
   // We keep a reference to any task we have scheduled so we can gracefully
   // force them to run if the syncer is trying to shutdown.
   Task* pending_work_;
-  Lock pending_work_lock_;
 
   // Set by the SyncCoreThread when Syncapi shutdown has completed and the
   // SyncerThread has terminated, so no more work will be scheduled. Read by
   // the UI thread in Stop().
   bool syncapi_has_shutdown_;
 
   // The BookmarkModel's home-sweet-home MessageLoop.
   MessageLoop* const bookmark_model_loop_;
 
+  // We use a Lock for all data members and a ConditionVariable to synchronize.
+  // We do this instead of using a WaitableEvent and a bool condition in order
+  // to guard against races that could arise due to the fact that the lack of a
+  // barrier permits instructions to be reordered by compiler optimizations.
+  // Possible or not, that route makes for very fragile code due to existence
+  // of theoretical races.
+  Lock lock_;
+
   // Used as a barrier at shutdown to ensure the SyncerThread terminates before
   // we allow the UI thread to return from Stop(). This gets signalled whenever
   // one of two events occur: a new pending_work_ task was scheduled, or the
   // SyncerThread has terminated. We only care about (1) when we are in Stop(),
   // because we have to manually Run() the task.
-  base::WaitableEvent syncapi_event_;
+  ConditionVariable syncapi_event_;
 
   DISALLOW_COPY_AND_ASSIGN(BookmarkModelWorker);
 };
 
 }  // namespace browser_sync
 
 #endif  // CHROME_BROWSER_SYNC_GLUE_BOOKMARK_MODEL_WORKER_H_