Bug 1287392 - Part 5: Fix bugs of sync for TaskTracer. r=cyu
authorThinker K.F. Li <thinker@codemud.net>
Tue, 22 Nov 2016 00:11:00 +0800
changeset 323917 4c3e92ce19904a458e91090569f9ca51538e3566
parent 323916 09620b9e6d069a45be1f7e2375acfad144b24f53
child 323918 01a2259e75ccb1132a4303fde2667340981eea2d
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewerscyu
bugs1287392
milestone53.0a1
Bug 1287392 - Part 5: Fix bugs of sync for TaskTracer. r=cyu
tools/profiler/tasktracer/GeckoTaskTracer.cpp
--- a/tools/profiler/tasktracer/GeckoTaskTracer.cpp
+++ b/tools/profiler/tasktracer/GeckoTaskTracer.cpp
@@ -148,83 +148,76 @@ inline static bool
 IsStartLogging()
 {
   return sStarted;
 }
 
 static void
 SetLogStarted(bool aIsStartLogging)
 {
-  MOZ_ASSERT(aIsStartLogging != IsStartLogging());
+  MOZ_ASSERT(aIsStartLogging != sStarted);
+  MOZ_ASSERT(sTraceInfos != nullptr);
   sStarted = aIsStartLogging;
 
   StaticMutexAutoLock lock(sMutex);
-  if (!aIsStartLogging) {
+  if (!aIsStartLogging && sTraceInfos) {
     for (uint32_t i = 0; i < sTraceInfos->Length(); ++i) {
       (*sTraceInfos)[i]->mObsolete = true;
     }
   }
 }
 
-static void
-CleanUp()
-{
-  MOZ_ASSERT(!IsStartLogging());
-  StaticMutexAutoLock lock(sMutex);
-
-  if (sTraceInfos) {
-    delete sTraceInfos;
-    sTraceInfos = nullptr;
-  }
-}
-
 inline static void
 ObsoleteCurrentTraceInfos()
 {
   // Note that we can't and don't need to acquire sMutex here because this
   // function is called before the other threads are recreated.
   for (uint32_t i = 0; i < sTraceInfos->Length(); ++i) {
     (*sTraceInfos)[i]->mObsolete = true;
   }
 }
 
 } // namespace anonymous
 
 nsCString*
 TraceInfo::AppendLog()
 {
-  MutexAutoLock lock(mLogsMutex);
   return mLogs.AppendElement();
 }
 
 void
 TraceInfo::MoveLogsInto(TraceInfoLogsType& aResult)
 {
   MutexAutoLock lock(mLogsMutex);
   aResult.AppendElements(Move(mLogs));
 }
 
 void
 InitTaskTracer(uint32_t aFlags)
 {
+  StaticMutexAutoLock lock(sMutex);
+
   if (aFlags & FORKED_AFTER_NUWA) {
     ObsoleteCurrentTraceInfos();
     return;
   }
 
   MOZ_ASSERT(!sTraceInfos);
-  sTraceInfos = new nsTArray<UniquePtr<TraceInfo>>();
 
   sTraceInfoTLS.init();
+  // A memory barrier is necessary here.
+  sTraceInfos = new nsTArray<UniquePtr<TraceInfo>>();
 }
 
 void
 ShutdownTaskTracer()
 {
-  CleanUp();
+  if (IsStartLogging()) {
+    SetLogStarted(false);
+  }
 }
 
 static void
 FreeTraceInfo(TraceInfo* aTraceInfo)
 {
   StaticMutexAutoLock lock(sMutex);
   if (aTraceInfo) {
     UniquePtr<TraceInfo> traceinfo(aTraceInfo);
@@ -238,17 +231,16 @@ FreeTraceInfo(TraceInfo* aTraceInfo)
 void FreeTraceInfo()
 {
   FreeTraceInfo(sTraceInfoTLS.get());
 }
 
 TraceInfo*
 GetOrCreateTraceInfo()
 {
-  ENSURE_TRUE(sTraceInfoTLS.init(), nullptr);
   ENSURE_TRUE(IsStartLogging(), nullptr);
 
   TraceInfo* info = sTraceInfoTLS.get();
   if (info && info->mObsolete) {
     // TraceInfo is obsolete: remove it.
     FreeTraceInfo(info);
     info = nullptr;
   }
@@ -320,61 +312,65 @@ LogDispatch(uint64_t aTaskId, uint64_t a
   TraceInfo* info = GetOrCreateTraceInfo();
   ENSURE_TRUE_VOID(info);
 
   // aDelayTimeMs is the expected delay time in milliseconds, thus the dispatch
   // time calculated of it might be slightly off in the real world.
   uint64_t time = (aDelayTimeMs <= 0) ? GetTimestamp() :
                   GetTimestamp() + aDelayTimeMs;
 
+  MutexAutoLock lock(info->mLogsMutex);
   // Log format:
   // [0 taskId dispatchTime sourceEventId sourceEventType parentTaskId]
   nsCString* log = info->AppendLog();
   if (log) {
     log->AppendPrintf("%d %lld %lld %lld %d %lld",
                       ACTION_DISPATCH, aTaskId, time, aSourceEventId,
                       aSourceEventType, aParentTaskId);
   }
 }
 
 void
 LogBegin(uint64_t aTaskId, uint64_t aSourceEventId)
 {
   TraceInfo* info = GetOrCreateTraceInfo();
   ENSURE_TRUE_VOID(info);
 
+  MutexAutoLock lock(info->mLogsMutex);
   // Log format:
   // [1 taskId beginTime processId threadId]
   nsCString* log = info->AppendLog();
   if (log) {
     log->AppendPrintf("%d %lld %lld %d %d",
                       ACTION_BEGIN, aTaskId, GetTimestamp(), getpid(), gettid());
   }
 }
 
 void
 LogEnd(uint64_t aTaskId, uint64_t aSourceEventId)
 {
   TraceInfo* info = GetOrCreateTraceInfo();
   ENSURE_TRUE_VOID(info);
 
+  MutexAutoLock lock(info->mLogsMutex);
   // Log format:
   // [2 taskId endTime]
   nsCString* log = info->AppendLog();
   if (log) {
     log->AppendPrintf("%d %lld %lld", ACTION_END, aTaskId, GetTimestamp());
   }
 }
 
 void
 LogVirtualTablePtr(uint64_t aTaskId, uint64_t aSourceEventId, uintptr_t* aVptr)
 {
   TraceInfo* info = GetOrCreateTraceInfo();
   ENSURE_TRUE_VOID(info);
 
+  MutexAutoLock lock(info->mLogsMutex);
   // Log format:
   // [4 taskId address]
   nsCString* log = info->AppendLog();
   if (log) {
     // Since addr2line used by SPS addon can not solve non-function
     // addresses, we use the first entry of vtable as the symbol to
     // solve.  We should find a better solution later.
     log->AppendPrintf("%d %lld %p", ACTION_GET_VTABLE, aTaskId, *aVptr);
@@ -397,16 +393,17 @@ void AddLabel(const char* aFormat, ...)
   ENSURE_TRUE_VOID(info);
 
   va_list args;
   va_start(args, aFormat);
   nsAutoCString buffer;
   buffer.AppendPrintf(aFormat, args);
   va_end(args);
 
+  MutexAutoLock lock(info->mLogsMutex);
   // Log format:
   // [3 taskId "label"]
   nsCString* log = info->AppendLog();
   if (log) {
     log->AppendPrintf("%d %lld %lld \"%s\"", ACTION_ADD_LABEL, info->mCurTaskId,
                       GetTimestamp(), buffer.get());
   }
 }