Bug 1371882 - MEMORYBLOCKCACHE_ERRORS telemetry to catch unexpected errors without crashing - r=cpearce,francois draft
authorGerald Squelart <gsquelart@mozilla.com>
Mon, 12 Jun 2017 16:22:28 +1200
changeset 595161 4b7e2d0829a8286be5f6d30841961db7882b488e
parent 595160 6fb8b5d967dc21b6ae7a6489f287508f0b3fed35
child 595162 e117063a8b99f6ca7cde6655569c70d51f98eea6
push id64265
push usergsquelart@mozilla.com
push dateFri, 16 Jun 2017 03:37:56 +0000
reviewerscpearce, francois
bugs1371882
milestone56.0a1
Bug 1371882 - MEMORYBLOCKCACHE_ERRORS telemetry to catch unexpected errors without crashing - r=cpearce,francois No errors are expected to happen in MemoryBlockCache (except a few 'InitAllocation', which would still be good to know about), but instead of taking drastic measures in these cases (i.e., crash), I would prefer to collect some telemetry first. MozReview-Commit-ID: 4WdFS34lgzj
dom/media/MemoryBlockCache.cpp
dom/media/MemoryBlockCache.h
toolkit/components/telemetry/Histograms.json
--- a/dom/media/MemoryBlockCache.cpp
+++ b/dom/media/MemoryBlockCache.cpp
@@ -10,21 +10,43 @@
 
 namespace mozilla {
 
 #undef LOG
 LazyLogModule gMemoryBlockCacheLog("MemoryBlockCache");
 #define LOG(x, ...)                                                            \
   MOZ_LOG(gMemoryBlockCacheLog, LogLevel::Debug, ("%p " x, this, ##__VA_ARGS__))
 
+enum MemoryBlockCacheTelemetryErrors
+{
+  // Don't change order/numbers! Add new values at the end and update
+  // MEMORYBLOCKCACHE_ERRORS description in Histograms.json.
+  InitUnderuse = 0,
+  InitAllocation = 1,
+  ReadOverrun = 2,
+  WriteBlockOverflow = 3,
+  WriteBlockCannotGrow = 4,
+  MoveBlockSourceOverrun = 5,
+  MoveBlockDestOverflow = 6,
+  MoveBlockCannotGrow = 7,
+};
+
 MemoryBlockCache::MemoryBlockCache(int64_t aContentLength)
   // Buffer whole blocks.
   : mInitialContentLength((aContentLength >= 0) ? size_t(aContentLength) : 0)
   , mMutex("MemoryBlockCache")
+  , mHasGrown(false)
 {
+  if (aContentLength <= 0) {
+    LOG("@%p MemoryBlockCache() "
+        "MEMORYBLOCKCACHE_ERRORS='InitUnderuse'",
+        this);
+    Telemetry::Accumulate(Telemetry::HistogramID::MEMORYBLOCKCACHE_ERRORS,
+                          InitUnderuse);
+  }
 }
 
 MemoryBlockCache::~MemoryBlockCache()
 {
   MOZ_ASSERT(mBuffer.IsEmpty());
 }
 
 bool
@@ -35,27 +57,38 @@ MemoryBlockCache::EnsureBufferCanContain
     return true;
   }
   size_t desiredLength = ((aContentLength - 1) / BLOCK_SIZE + 1) * BLOCK_SIZE;
   if (mBuffer.Length() >= desiredLength) {
     // Already large enough.
     return true;
   }
   // Need larger buffer, attempt to re-allocate.
-  return mBuffer.SetLength(desiredLength, mozilla::fallible);
+  if (!mBuffer.SetLength(desiredLength, mozilla::fallible)) {
+    return false;
+  }
+  mHasGrown = false;
+  return true;
 }
 
 nsresult
 MemoryBlockCache::Init()
 {
   LOG("@%p Init()", this);
   MutexAutoLock lock(mMutex);
   // Attempt to pre-allocate buffer for expected content length.
-  return EnsureBufferCanContain(mInitialContentLength) ? NS_OK
-                                                       : NS_ERROR_FAILURE;
+  if (!EnsureBufferCanContain(mInitialContentLength)) {
+    LOG("@%p Init() MEMORYBLOCKCACHE_ERRORS='InitAllocation'", this);
+    Telemetry::Accumulate(Telemetry::HistogramID::MEMORYBLOCKCACHE_ERRORS,
+                          InitAllocation);
+    return NS_ERROR_FAILURE;
+  }
+  // Ignore initial growth.
+  mHasGrown = false;
+  return NS_OK;
 }
 
 void
 MemoryBlockCache::Close()
 {
   LOG("@%p Close()", this);
   MutexAutoLock lock(mMutex);
   mBuffer.SetLength(0);
@@ -64,17 +97,30 @@ MemoryBlockCache::Close()
 nsresult
 MemoryBlockCache::WriteBlock(uint32_t aBlockIndex,
                              Span<const uint8_t> aData1,
                              Span<const uint8_t> aData2)
 {
   MutexAutoLock lock(mMutex);
 
   size_t offset = BlockIndexToOffset(aBlockIndex);
+  if (offset + aData1.Length() + aData2.Length() > mBuffer.Length() &&
+      !mHasGrown) {
+    LOG("@%p WriteBlock() "
+        "MEMORYBLOCKCACHE_ERRORS='WriteBlockOverflow'",
+        this);
+    Telemetry::Accumulate(Telemetry::HistogramID::MEMORYBLOCKCACHE_ERRORS,
+                          WriteBlockOverflow);
+  }
   if (!EnsureBufferCanContain(offset + aData1.Length() + aData2.Length())) {
+    LOG("%p WriteBlock() "
+        "MEMORYBLOCKCACHE_ERRORS='WriteBlockCannotGrow'",
+        this);
+    Telemetry::Accumulate(Telemetry::HistogramID::MEMORYBLOCKCACHE_ERRORS,
+                          WriteBlockCannotGrow);
     return NS_ERROR_FAILURE;
   }
 
   memcpy(mBuffer.Elements() + offset, aData1.Elements(), aData1.Length());
   if (aData2.Length() > 0) {
     memcpy(mBuffer.Elements() + offset + aData1.Length(),
            aData2.Elements(),
            aData2.Length());
@@ -88,34 +134,56 @@ MemoryBlockCache::Read(int64_t aOffset,
                        uint8_t* aData,
                        int32_t aLength,
                        int32_t* aBytes)
 {
   MutexAutoLock lock(mMutex);
 
   MOZ_ASSERT(aOffset >= 0);
   if (aOffset + aLength > int64_t(mBuffer.Length())) {
+    LOG("@%p Read() MEMORYBLOCKCACHE_ERRORS='ReadOverrun'", this);
+    Telemetry::Accumulate(Telemetry::HistogramID::MEMORYBLOCKCACHE_ERRORS,
+                          ReadOverrun);
     return NS_ERROR_FAILURE;
   }
 
   memcpy(aData, mBuffer.Elements() + aOffset, aLength);
   *aBytes = aLength;
 
   return NS_OK;
 }
 
 nsresult
 MemoryBlockCache::MoveBlock(int32_t aSourceBlockIndex, int32_t aDestBlockIndex)
 {
   MutexAutoLock lock(mMutex);
 
   size_t sourceOffset = BlockIndexToOffset(aSourceBlockIndex);
   size_t destOffset = BlockIndexToOffset(aDestBlockIndex);
-  if (sourceOffset + BLOCK_SIZE > mBuffer.Length() ||
-      !EnsureBufferCanContain(destOffset + BLOCK_SIZE)) {
+  if (sourceOffset + BLOCK_SIZE > mBuffer.Length()) {
+    LOG("@%p MoveBlock() "
+        "MEMORYBLOCKCACHE_ERRORS='MoveBlockSourceOverrun'",
+        this);
+    Telemetry::Accumulate(Telemetry::HistogramID::MEMORYBLOCKCACHE_ERRORS,
+                          MoveBlockSourceOverrun);
+    return NS_ERROR_FAILURE;
+  }
+  if (destOffset + BLOCK_SIZE > mBuffer.Length() && !mHasGrown) {
+    LOG("@%p MoveBlock() "
+        "MEMORYBLOCKCACHE_ERRORS='MoveBlockDestOverflow'",
+        this);
+    Telemetry::Accumulate(Telemetry::HistogramID::MEMORYBLOCKCACHE_ERRORS,
+                          MoveBlockDestOverflow);
+  }
+  if (!EnsureBufferCanContain(destOffset + BLOCK_SIZE)) {
+    LOG("@%p MoveBlock() "
+        "MEMORYBLOCKCACHE_ERRORS='MoveBlockCannotGrow'",
+        this);
+    Telemetry::Accumulate(Telemetry::HistogramID::MEMORYBLOCKCACHE_ERRORS,
+                          MoveBlockCannotGrow);
     return NS_ERROR_FAILURE;
   }
 
   memcpy(mBuffer.Elements() + destOffset,
          mBuffer.Elements() + sourceOffset,
          BLOCK_SIZE);
 
   return NS_OK;
--- a/dom/media/MemoryBlockCache.h
+++ b/dom/media/MemoryBlockCache.h
@@ -72,13 +72,14 @@ private:
 
   // Initial content length.
   const size_t mInitialContentLength;
 
   // Mutex which controls access to all members below.
   Mutex mMutex;
 
   nsTArray<uint8_t> mBuffer;
+  bool mHasGrown;
 };
 
 } // End namespace mozilla.
 
 #endif /* MEMORY_BLOCK_CACHE_H_ */
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -8498,16 +8498,25 @@
     "alert_emails": ["gsquelart@mozilla.com"],
     "bug_numbers": [1371205],
     "expires_in_version": "60",
     "kind": "linear",
     "high": 16777216,
     "n_buckets": 66,
     "description": "MediaCacheStream stream notified length size in bytes, from the HTTP header. Recorded when each MediaCacheStream is first notified."
   },
+  "MEMORYBLOCKCACHE_ERRORS": {
+    "record_in_processes": ["main", "content"],
+    "alert_emails": ["gsquelart@mozilla.com"],
+    "bug_numbers": [1371882],
+    "expires_in_version": "60",
+    "kind": "enumerated",
+    "n_values": 16,
+    "description": "Unexpected errors encountered during use of MemoryBlockCache. 0=InitUnderuse, 1=InitAllocation, 2=ReadOverrun, 3=WriteBlockOverflow, 4=WriteBlockCannotGrow, 5=MoveBlockSourceOverrun, 6=MoveBlockDestOverflow, 7=MoveBlockCannotGrow"
+  },
   "VIDEO_MFT_OUTPUT_NULL_SAMPLES": {
     "record_in_processes": ["main", "content"],
     "alert_emails": ["cpearce@mozilla.com"],
     "expires_in_version": "53",
     "kind": "enumerated",
     "n_values": 10,
     "description": "Does the WMF video decoder return success but null output? 0 = playback successful, 1 = excessive null output but able to decode some frames, 2 = excessive null output and gave up, 3 = null output but recovered, 4 = non-excessive null output without being able to decode frames.",
     "bug_numbers": [1176071]