Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(243)

Unified Diff: components/history/core/browser/history_model_worker.h

Issue 2757193003: [Sync] Do not deadlock when joining sync thread with a pending HistoryModelWorker task. (Closed)
Patch Set: self-review Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/history/core/browser/history_model_worker.h
diff --git a/components/history/core/browser/history_model_worker.h b/components/history/core/browser/history_model_worker.h
index 6421c64d3a198d1080cc5ccb00a04d219dbcea03..b945afc25b41d5a5b498154d3511b73f27c4263d 100644
--- a/components/history/core/browser/history_model_worker.h
+++ b/components/history/core/browser/history_model_worker.h
@@ -11,6 +11,8 @@
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/single_thread_task_runner.h"
+#include "base/synchronization/lock.h"
+#include "base/synchronization/waitable_event.h"
#include "base/task/cancelable_task_tracker.h"
#include "components/sync/engine/model_safe_worker.h"
@@ -29,6 +31,7 @@ class HistoryModelWorker : public syncer::ModelSafeWorker {
const scoped_refptr<base::SingleThreadTaskRunner>& ui_thread);
// syncer::ModelSafeWorker implementation.
+ void RequestStop() override;
syncer::ModelSafeGroup GetModelSafeGroup() override;
bool IsOnModelThread() override;
@@ -37,17 +40,45 @@ class HistoryModelWorker : public syncer::ModelSafeWorker {
const syncer::WorkCallback& work) override;
private:
+ class WorkerTask;
+
~HistoryModelWorker() override;
+ // Called before a WorkCallback starts running. If this returns false, the
maxbogue 2017/03/20 16:52:37 nit: mention the side effect of this function some
fdoray 2017/03/20 18:54:54 n/a
+ // WorkCallback must not run.
+ bool WillRunWorkCallback();
+
+ // Called after a WorkCallback has run.
+ void DidRunWorkCallback();
+
const base::WeakPtr<history::HistoryService> history_service_;
// A reference to the UI thread's task runner.
const scoped_refptr<base::SingleThreadTaskRunner> ui_thread_;
+ // Signaled when a task posted by DoWorkAndWaitUntilDoneImpl() is deleted,
+ // i.e. after it runs or when it is abandoned. Reset at the beginning of every
+ // DoWorkAndWaitUntilDoneImpl() call.
+ base::WaitableEvent work_done_or_abandoned_;
+
+ // Signaled to unblock DoWorkAndWaitUntilDoneImpl() and prevent any
+ // WorkCallback from being scheduled when RequestStop() is called and there is
+ // no pending task from this worker on the history DB thread.
+ base::WaitableEvent stop_requested_;
+
+ // True if a WorkCallback posted by this worker is currently running.
+ bool work_callback_running_ = false;
+
+ // Synchronizes access to |stop_requested_| and |work_callback_running_|.
+ base::Lock lock_;
+
// Helper object to make sure we don't leave tasks running on the history
// thread.
std::unique_ptr<base::CancelableTaskTracker> cancelable_tracker_;
+ // Verifies that calls to DoWorkAndWaitUntilDoneImpl() are sequenced.
+ base::SequenceChecker sequence_checker_;
+
DISALLOW_COPY_AND_ASSIGN(HistoryModelWorker);
};

Powered by Google App Engine
This is Rietveld 408576698