diff options
| author | tim@chromium.org <tim@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-17 15:35:14 +0000 |
|---|---|---|
| committer | tim@chromium.org <tim@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-17 15:35:14 +0000 |
| commit | 5b4fda28ba487bc01b83b1d32ab3988dac9270c2 (patch) | |
| tree | 74a3fb325b1e7ac2376b7f9598c2fac83b26f48a /chrome | |
| parent | 49c6319f3f5881d16c9a82f2350e46dd7ee05b7f (diff) | |
| download | chromium_src-5b4fda28ba487bc01b83b1d32ab3988dac9270c2.zip chromium_src-5b4fda28ba487bc01b83b1d32ab3988dac9270c2.tar.gz chromium_src-5b4fda28ba487bc01b83b1d32ab3988dac9270c2.tar.bz2 | |
Add a lock to read |state_| in UIModelWorker::DoWorkAndWait. We don't
strictly need to acquire the lock here because this can only get called
while !syncapi_has_shutdown_ and STOPPED could only get set after that
bool is false (and that bool is protected by locks), the purpose of this
DCHECK/return is for catching developer error in extraordinary cases,
so in a way, adding ANNOTATE_UNPROTECTED_READ would seem hypocritical.
BUG=32892
TEST=UIModelWorkerTest
Review URL: http://codereview.chromium.org/965002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@41840 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
| -rw-r--r-- | chrome/browser/sync/glue/ui_model_worker.cc | 12 | ||||
| -rw-r--r-- | chrome/browser/sync/glue/ui_model_worker.h | 4 |
2 files changed, 8 insertions, 8 deletions
diff --git a/chrome/browser/sync/glue/ui_model_worker.cc b/chrome/browser/sync/glue/ui_model_worker.cc index d11ad5b..89a9c43 100644 --- a/chrome/browser/sync/glue/ui_model_worker.cc +++ b/chrome/browser/sync/glue/ui_model_worker.cc @@ -4,19 +4,21 @@ #include "chrome/browser/sync/glue/ui_model_worker.h" +#include "base/dynamic_annotations.h" #include "base/message_loop.h" #include "base/waitable_event.h" namespace browser_sync { void UIModelWorker::DoWorkAndWaitUntilDone(Closure* work) { - // It is possible this gets called when we are in the STOPPING state, because + // In most cases, this method is called in WORKING state. It is possible this + // gets called when we are in the RUNNING_MANUAL_SHUTDOWN_PUMP state, because // the UI loop has initiated shutdown but the syncer hasn't got the memo yet. // This is fine, the work will get scheduled and run normally or run by our - // code handling this case in Stop(). - DCHECK_NE(state_, STOPPED); - if (state_ == STOPPED) - return; + // code handling this case in Stop(). Note there _no_ way we can be in here + // with state_ = STOPPED, so it is safe to read / compare in this case. + CHECK_NE(ANNOTATE_UNPROTECTED_READ(state_), STOPPED); + if (MessageLoop::current() == ui_loop_) { DLOG(WARNING) << "DoWorkAndWaitUntilDone called from " << "ui_loop_. Probably a nested invocation?"; diff --git a/chrome/browser/sync/glue/ui_model_worker.h b/chrome/browser/sync/glue/ui_model_worker.h index de87795..84833d5 100644 --- a/chrome/browser/sync/glue/ui_model_worker.h +++ b/chrome/browser/sync/glue/ui_model_worker.h @@ -99,9 +99,7 @@ class UIModelWorker : public browser_sync::ModelSafeWorker { }; // 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). + // read this value from other threads when you know it is absolutely safe. State state_; // We keep a reference to any task we have scheduled so we can gracefully |