Ensure Origin Thread Affinity for BlameContext

base/trace_event/blame_context.cc

Index: base/trace_event/blame_context.cc
diff --git a/base/trace_event/blame_context.cc b/base/trace_event/blame_context.cc
index 27d2d2eb7509c2b57f161aa5263c2787cc337a3c..74b4a09b602086f6bb513235c4945460d92e1e58 100644
--- a/base/trace_event/blame_context.cc
+++ b/base/trace_event/blame_context.cc
@@ -5,6 +5,7 @@
 #include "base/trace_event/blame_context.h"
 
 #include "base/strings/stringprintf.h"
+#include "base/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/trace_event_argument.h"
 
@@ -24,7 +25,9 @@ BlameContext::BlameContext(const char* category,
       id_(id),
       parent_scope_(parent_context ? parent_context->scope() : nullptr),
       parent_id_(parent_context ? parent_context->id() : 0),
-      category_group_enabled_(nullptr) {
+      category_group_enabled_(nullptr),
+      task_runner_(ThreadTaskRunnerHandle::Get()),
+      weak_factory_(this) {
   DCHECK(!parent_context || !std::strcmp(name_, parent_context->name()))
       << "Parent blame context must have the same name";
 }
@@ -54,6 +57,7 @@ void BlameContext::Leave() {
 }
 
 void BlameContext::TakeSnapshot() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(WasInitialized());
   if (!*category_group_enabled_)
     return;
@@ -71,9 +75,18 @@ void BlameContext::TakeSnapshot() {
                                   arg_values, TRACE_EVENT_FLAG_HAS_ID);
 }
 
+// Called by TraceLog from a thread possibly different from the one where this
+// BlameContext instance was constructed. Since any access to TraceLog's list
+// of observers is lock-guarded, we can access members of BlameContext from a
+// different thread via a raw pointer safely without worrying about threading
+// issues or pointer validity.
 void BlameContext::OnTraceLogEnabled() {
   DCHECK(WasInitialized());
-  TakeSnapshot();
+  if (task_runner_->BelongsToCurrentThread())
+    TakeSnapshot();
+  else
+    task_runner_->PostTask(FROM_HERE, Bind(&BlameContext::TakeSnapshot,

[Primiano Tucci (use gerrit), 2016/04/26 15:32:15]

What happens in the case that BlameContext is destroyed on another thread while
you are executing this? You are going to access a weak_factory_ that is being
torn down.
I think you just moved the race to this method now.

In general, this method can be called while the BlameContext is destroyed. The
EnabledStateObserver does not manage the lifetime of the observer (BlameContext
in this case).

[Sami, 2016/04/26 15:45:26]

BlameContext removes itself as an observer in the destructor (which I presume is
a thread safe operation), so it doesn't seem like there's a way for this method
to execute after BlameContext has gone away or while it is being destructed.

+                                           weak_factory_.GetWeakPtr()));
 }
 
 void BlameContext::OnTraceLogDisabled() {}