Ensure Origin Thread Affinity for BlameContext

base/trace_event/blame_context.cc

 // Copyright 2016 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.
 
 #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"
 
 namespace base {
 namespace trace_event {
 
 BlameContext::BlameContext(const char* category,
                            const char* name,
                            const char* type,
                            const char* scope,
                            int64_t id,
                            const BlameContext* parent_context)
     : category_(category),
       name_(name),
       type_(type),
       scope_(scope),
       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";
 }
 
 BlameContext::~BlameContext() {
   DCHECK(WasInitialized());
   TRACE_EVENT_API_ADD_TRACE_EVENT(
       TRACE_EVENT_PHASE_DELETE_OBJECT, category_group_enabled_, type_, scope_,
       id_, 0, nullptr, nullptr, nullptr, nullptr, TRACE_EVENT_FLAG_HAS_ID);
   trace_event::TraceLog::GetInstance()->RemoveEnabledStateObserver(this);
 }
 
 void BlameContext::Enter() {
   DCHECK(WasInitialized());
   TRACE_EVENT_API_ADD_TRACE_EVENT(TRACE_EVENT_PHASE_ENTER_CONTEXT,
                                   category_group_enabled_, name_, scope_, id_,
                                   0 /* num_args */, nullptr, nullptr, nullptr,
                                   nullptr, TRACE_EVENT_FLAG_HAS_ID);
 }
 
 void BlameContext::Leave() {
   DCHECK(WasInitialized());
   TRACE_EVENT_API_ADD_TRACE_EVENT(TRACE_EVENT_PHASE_LEAVE_CONTEXT,
                                   category_group_enabled_, name_, scope_, id_,
                                   0 /* num_args */, nullptr, nullptr, nullptr,
                                   nullptr, TRACE_EVENT_FLAG_HAS_ID);
 }
 
 void BlameContext::TakeSnapshot() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(WasInitialized());
   if (!*category_group_enabled_)
     return;
   std::unique_ptr<trace_event::TracedValue> snapshot(
       new trace_event::TracedValue);
   AsValueInto(snapshot.get());
   static const char* kArgName = "snapshot";
   const int kNumArgs = 1;
   unsigned char arg_types[1] = {TRACE_VALUE_TYPE_CONVERTABLE};
   std::unique_ptr<trace_event::ConvertableToTraceFormat> arg_values[1] = {
       std::move(snapshot)};
   TRACE_EVENT_API_ADD_TRACE_EVENT(TRACE_EVENT_PHASE_SNAPSHOT_OBJECT,
                                   category_group_enabled_, type_, scope_, id_,
                                   kNumArgs, &kArgName, arg_types, nullptr,
                                   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() {}
 
 void BlameContext::AsValueInto(trace_event::TracedValue* state) {
   DCHECK(WasInitialized());
   if (!parent_id_)
     return;
   state->BeginDictionary("parent");
   state->SetString("id_ref", StringPrintf("0x%" PRIx64, parent_id_));
@@ skipping 10 matching lines @@
   trace_event::TraceLog::GetInstance()->AddEnabledStateObserver(this);
   TakeSnapshot();
 }
 
 bool BlameContext::WasInitialized() const {
   return category_group_enabled_ != nullptr;
 }
 
 }  // namespace trace_event
 }  // namespace base