Commit ac126c97 authored by valenting's avatar valenting
Browse files

Bug 1755186 - HpackDynamicTableReporter::CollectReports should hold a mutex a=diannaS 

HpackDynamicTableReporter::mMutex protects access to HpackDynamicTableReporter::mCompressor
nvFIFO::mMutex protects access to mvFIFO::mTable - is aquired when adding or removing elements in the table, and when reporting SizeOfDynamicTable

Original Revision: https://phabricator.services.mozilla.com/D200814

Differential Revision: https://phabricator.services.mozilla.com/D202906
parent 46c61567

netwerk/protocol/http/Http2Compression.cpp

+24 −9
Original line number Diff line number Diff line
@@ -61,6 +61,7 @@ class HpackDynamicTableReporter final : public nsIMemoryReporter { 
  NS_IMETHOD

  CollectReports(nsIHandleReportCallback* aHandleReport, nsISupports* aData,

                 bool aAnonymize) override {

    MutexAutoLock lock(mMutex);

    if (mCompressor) {

      MOZ_COLLECT_REPORT("explicit/network/hpack/dynamic-tables", KIND_HEAP,

                         UNITS_BYTES,
@@ -75,7 +76,8 @@ class HpackDynamicTableReporter final : public nsIMemoryReporter { 


  ~HpackDynamicTableReporter() = default;



  Http2BaseCompressor* mCompressor;

  Mutex mMutex{"HpackDynamicTableReporter"};

  Http2BaseCompressor* mCompressor MOZ_GUARDED_BY(mMutex);



  friend class Http2BaseCompressor;

};
@@ -187,13 +189,18 @@ nvFIFO::~nvFIFO() { Clear(); } 
void nvFIFO::AddElement(const nsCString& name, const nsCString& value) {

  nvPair* pair = new nvPair(name, value);

  mByteCount += pair->Size();

  MutexAutoLock lock(mMutex);

  mTable.PushFront(pair);

}



void nvFIFO::AddElement(const nsCString& name) { AddElement(name, ""_ns); }



void nvFIFO::RemoveElement() {

  nvPair* pair = mTable.Pop();

  nvPair* pair = nullptr;

  {

    MutexAutoLock lock(mMutex);

    pair = mTable.Pop();

  }

  if (pair) {

    mByteCount -= pair->Size();

    delete pair;
@@ -212,6 +219,7 @@ size_t nvFIFO::StaticLength() const { return gStaticHeaders->GetSize(); } 


void nvFIFO::Clear() {

  mByteCount = 0;

  MutexAutoLock lock(mMutex);

  while (mTable.GetSize()) {

    delete mTable.Pop();

  }
@@ -244,20 +252,27 @@ Http2BaseCompressor::~Http2BaseCompressor() { 
    Telemetry::Accumulate(mPeakCountID, mPeakCount);

  }

  UnregisterStrongMemoryReporter(mDynamicReporter);

  {

    MutexAutoLock lock(mDynamicReporter->mMutex);

    mDynamicReporter->mCompressor = nullptr;

  }

  mDynamicReporter = nullptr;

}



size_t nvFIFO::SizeOfDynamicTable(mozilla::MallocSizeOf aMallocSizeOf) const {

  size_t size = 0;

  MutexAutoLock lock(mMutex);

  for (const auto elem : mTable) {

    size += elem->SizeOfIncludingThis(aMallocSizeOf);

  }

  return size;

}



void Http2BaseCompressor::ClearHeaderTable() { mHeaderTable.Clear(); }



size_t Http2BaseCompressor::SizeOfExcludingThis(

    mozilla::MallocSizeOf aMallocSizeOf) const {

  size_t size = 0;

  for (uint32_t i = mHeaderTable.StaticLength(); i < mHeaderTable.Length();

       ++i) {

    size += mHeaderTable[i]->SizeOfIncludingThis(aMallocSizeOf);

  }

  return size;

  return mHeaderTable.SizeOfDynamicTable(aMallocSizeOf);

}



void Http2BaseCompressor::MakeRoom(uint32_t amount, const char* direction) {

netwerk/protocol/http/Http2Compression.h

+9 −0
Original line number Diff line number Diff line
@@ -13,6 +13,7 @@ 
#include "nsDeque.h"

#include "nsString.h"

#include "mozilla/Telemetry.h"

#include "mozilla/Mutex.h"



namespace mozilla {

namespace net {
@@ -47,10 +48,18 @@ class nvFIFO { 
  size_t StaticLength() const;

  void Clear();

  const nvPair* operator[](size_t index) const;

  size_t SizeOfDynamicTable(mozilla::MallocSizeOf aMallocSizeOf) const;



 private:

  uint32_t mByteCount{0};

  nsDeque<nvPair> mTable;



  // This mutex is held when adding or removing elements in the table

  // and when accessing the table from the main thread (in SizeOfDynamicTable)

  // Since the operator[] and other const methods are always called

  // on the socket thread, they don't need to lock the mutex.

  // Mutable so it can be locked in SizeOfDynamicTable which is const

  mutable Mutex mMutex MOZ_UNANNOTATED{"nvFIFO"};

};



class HpackDynamicTableReporter;