Status: Untriaged
Owner: [email protected]
CC: [email protected]
Labels: Type-Bug Pri-2 OS-All Area-Misc Valgrind ThreadSanitizer

New issue 25915 by [email protected]: Data races in BookmarkModelWorker
http://code.google.com/p/chromium/issues/detail?id=25915

There are at least two data races in this class:
on "syncapi_has_shutdown_" and on "state_".
Both data races were found by ThreadSanitizer, the report is attached.

(all the source code below is taken from
chrome/browser/sync/glue/bookmark_model_worker.cc)

Regarding the date race on "syncapi_has_shutdown_":
  49 void BookmarkModelWorker::OnSyncerShutdownComplete() {
  ...
  57   syncapi_has_shutdown_ = true;
  58   syncapi_event_.Signal();
  59 }

races with
  61 void BookmarkModelWorker::Stop() {
  ...
  69   // Drain any final task manually until the SyncerThread tells us it
has
  70   // totally finished. Note we use a 'while' loop and not 'if'. The
main subtle
  71   // reason for this is that syncapi_event could be signaled the
first time we
  72   // come through due to an old CallDoWork call, and we need to keep
looping
  73   // until the SyncerThread either calls it again or tells us it is
done. There
  74   // should only ever be 0 or 1 tasks Run() here, however.
  75   while (!syncapi_has_shutdown_) {
  76     {
  77       AutoLock lock(pending_work_lock_);
  78       if (pending_work_)
  79         pending_work_->Run();
  80     }
  81     syncapi_event_.Wait();  // Signaled either by new task, or
SyncerThread
  82                             // termination.
  83   }
  ...
  86 }

both accesses are not guarded by locks and no compiler barriers are used.
Similar data race is described at
http://code.google.com/p/data-race-test/wiki/PopularDataRaces

As for the data race on "state_", it is exactly the one described at
http://code.google.com/p/data-race-test/wiki/PopularDataRaces :

  14 void BookmarkModelWorker::CallDoWorkFromModelSafeThreadAndWait(
  15     ModelSafeWorkerInterface::Visitor* visitor) {
  16   // It is possible this gets called when we are in the STOPPING
state, because
  17   // the UI loop has initiated shutdown but the syncer hasn't got the
memo yet.
  18   // This is fine, the work will get scheduled and run normally or
run by our
  19   // code handling this case in Stop().
  20   DCHECK_NE(state_, STOPPED);
  21   if (state_ == STOPPED)
  22     return;
  ...

races with

  61 void BookmarkModelWorker::Stop() {
  62   DCHECK_EQ(MessageLoop::current(), bookmark_model_loop_);
  63   DCHECK_EQ(state_, WORKING);
  64
  65   // We're on our own now, the beloved UI MessageLoop is no longer
running.
  66   // Any tasks scheduled or to be scheduled on the UI MessageLoop
will not run.
  67   state_ = RUNNING_MANUAL_SHUTDOWN_PUMP;
  ...
  75   while (!syncapi_has_shutdown_) {
  ...
  83   }
  84
  85   state_ = STOPPED;
  86 }


Attachments:
        BookmarkModelWorker reports.txt  9.8 KB

--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings

--~--~---------~--~----~------------~-------~--~----~
Automated mail from issue updates at http://crbug.com/
Subscription options: http://groups.google.com/group/chromium-bugs
-~----------~----~----~----~------~----~------~--~---

Reply via email to