Bug 1536129 - Fix !MOZ_CALLSTACK_DISABLED blocking resource acquisition checking, r=erahm
authorHonza Bambas <honzab.moz@firemni.cz>
Tue, 19 Mar 2019 17:12:42 +0000
changeset 465088 505c7972935b38db9a9e406d8cb15d1c0abe5b36
parent 465087 4dedf93bfe2d448fe36697e8866d20fc69cfb6b4
child 465089 195cd323a4830c9a17e35821d9e3c86ce62e07c9
push id35732
push useropoprus@mozilla.com
push dateWed, 20 Mar 2019 10:52:37 +0000
treeherdermozilla-central@708979f9c3f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm
bugs1536129
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1536129 - Fix !MOZ_CALLSTACK_DISABLED blocking resource acquisition checking, r=erahm Differential Revision: https://phabricator.services.mozilla.com/D23882
xpcom/threads/BlockingResourceBase.cpp
xpcom/threads/BlockingResourceBase.h
--- a/xpcom/threads/BlockingResourceBase.cpp
+++ b/xpcom/threads/BlockingResourceBase.cpp
@@ -47,30 +47,35 @@ PRCallOnceType BlockingResourceBase::sCa
 MOZ_THREAD_LOCAL(BlockingResourceBase*)
 BlockingResourceBase::sResourceAcqnChainFront;
 BlockingResourceBase::DDT* BlockingResourceBase::sDeadlockDetector;
 
 void BlockingResourceBase::StackWalkCallback(uint32_t aFrameNumber, void* aPc,
                                              void* aSp, void* aClosure) {
 #  ifndef MOZ_CALLSTACK_DISABLED
   AcquisitionState* state = (AcquisitionState*)aClosure;
-  state->AppendElement(aPc);
+  state->ref().AppendElement(aPc);
 #  endif
 }
 
 void BlockingResourceBase::GetStackTrace(AcquisitionState& aState) {
 #  ifndef MOZ_CALLSTACK_DISABLED
   // Skip this function and the calling function.
   const uint32_t kSkipFrames = 2;
 
-  aState.Clear();
+  // Clear the array...
+  aState.reset();
+  // ...and create a new one; this also puts the state to 'acquired' status
+  // regardless of whether we obtain a stack trace or not.
+  aState.emplace();
 
   // NB: Ignore the return value, there's nothing useful we can do if this
   //     this fails.
-  MozStackWalk(StackWalkCallback, kSkipFrames, 24, &aState);
+  MozStackWalk(StackWalkCallback, kSkipFrames, kAcquisitionStateStackSize,
+               aState.ptr());
 #  endif
 }
 
 /**
  * PrintCycle
  * Append to |aOut| detailed information about the circular
  * dependency in |aCycle|.  Returns true if it *appears* that this
  * cycle may represent an imminent deadlock, but this is merely a
@@ -168,20 +173,20 @@ bool BlockingResourceBase::Print(nsACStr
   fputs(" calling context\n", stderr);
 #  ifdef MOZ_CALLSTACK_DISABLED
   fputs("  [stack trace unavailable]\n", stderr);
 #  else
   const AcquisitionState& state = acquired ? mAcquired : mFirstSeen;
 
   WalkTheStackCodeAddressService addressService;
 
-  for (uint32_t i = 0; i < state.Length(); i++) {
+  for (uint32_t i = 0; i < state.ref().Length(); i++) {
     const size_t kMaxLength = 1024;
     char buffer[kMaxLength];
-    addressService.GetLocation(i + 1, state[i], buffer, kMaxLength);
+    addressService.GetLocation(i + 1, state.ref()[i], buffer, kMaxLength);
     const char* fmt = "    %s\n";
     aOut.AppendLiteral("    ");
     aOut.Append(buffer);
     aOut.AppendLiteral("\n");
     fprintf(stderr, fmt, buffer);
   }
 
 #  endif
@@ -293,17 +298,19 @@ void BlockingResourceBase::Acquire() {
 
   ResourceChainAppend(ResourceChainFront());
 
 #  ifdef MOZ_CALLSTACK_DISABLED
   mAcquired = true;
 #  else
   // Take a stack snapshot.
   GetStackTrace(mAcquired);
-  if (mFirstSeen.IsEmpty()) {
+  MOZ_ASSERT(IsAcquired());
+
+  if (!mFirstSeen) {
     mFirstSeen = mAcquired;
   }
 #  endif
 }
 
 void BlockingResourceBase::Release() {
   if (mType == eCondVar) {
     MOZ_ASSERT_UNREACHABLE(
--- a/xpcom/threads/BlockingResourceBase.h
+++ b/xpcom/threads/BlockingResourceBase.h
@@ -96,17 +96,22 @@ class BlockingResourceBase {
 
   // ``DDT'' = ``Deadlock Detector Type''
   typedef DeadlockDetector<BlockingResourceBase> DDT;
 
  protected:
 #  ifdef MOZ_CALLSTACK_DISABLED
   typedef bool AcquisitionState;
 #  else
-  typedef AutoTArray<void*, 24> AcquisitionState;
+  // Using maybe to use emplacement as the acquisition state flag; we may not
+  // always get a stack trace because of possible stack walk suppression or
+  // errors, hence can't use !IsEmpty() on the array itself as indication.
+  static size_t const kAcquisitionStateStackSize = 24;
+  typedef Maybe<AutoTArray<void*, kAcquisitionStateStackSize> >
+      AcquisitionState;
 #  endif
 
   /**
    * BlockingResourceBase
    * Initialize this blocking resource.  Also hooks the resource into
    * instrumentation code.
    *
    * Thread safe.
@@ -212,33 +217,27 @@ class BlockingResourceBase {
    * Indicate this resource is not acquired.
    *
    * *NOT* thread safe.  Requires ownership of underlying resource.
    */
   void ClearAcquisitionState() {
 #  ifdef MOZ_CALLSTACK_DISABLED
     mAcquired = false;
 #  else
-    mAcquired.Clear();
+    mAcquired.reset();
 #  endif
   }
 
   /**
    * IsAcquired
    * Indicates if this resource is acquired.
    *
    * *NOT* thread safe.  Requires ownership of underlying resource.
    */
-  bool IsAcquired() const {
-#  ifdef MOZ_CALLSTACK_DISABLED
-    return mAcquired;
-#  else
-    return !mAcquired.IsEmpty();
-#  endif
-  }
+  bool IsAcquired() const { return (bool)mAcquired; }
 
   /**
    * mChainPrev
    * A series of resource acquisitions creates a chain of orders.  This
    * chain is implemented as a linked list; |mChainPrev| points to the
    * resource most recently Acquire()'d before this one.
    **/
   BlockingResourceBase* mChainPrev;