Commit ed8cdf8a authored by Kris Maglione's avatar Kris Maglione
Browse files

Bug 1563825: Don't assert on failure to decode message after OOM. r=mccr8 

Ideally, we don't want to continue running a child process if it fails to
handle a message from the parent, since that could mean child and parent state
could get out of sync. But since this assertion is only a diagnostic assert,
it isn't guaranteeing that in release builds anyway. And since the vast
majority of the crashes we are seeing in builds with diagnostic asserts
enabled appear to be OOMs, we can't really use crash reports to diagnose other
issues.

Ideally (again), we'd determine if the failure was caused by an OOM based on
the failure code returned by the structured clone decode call. Unfortunately,
though, since the spec requires that we return a generic `DataCloneError` on
failure, the structured clone code intentionally hides the specifics of
failure from callers. Propagating out more specific failure reasons for use by
privileged callers is nontrivial. So this patch essentially does the same
thing as crash reports do, and checks whether an OOM was reported recently,
and hasn't been recovered from by a successful GC.

Differential Revision: https://phabricator.services.mozilla.com/D176288
parent 95a02539

dom/ipc/jsactor/JSActorManager.cpp

+3 −1
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ 
#include "mozilla/dom/PWindowGlobal.h"

#include "mozilla/ipc/ProtocolUtils.h"

#include "mozilla/AppShutdown.h"

#include "mozilla/CycleCollectedJSRuntime.h"

#include "mozilla/ScopeExit.h"

#include "mozJSModuleLoader.h"

#include "jsapi.h"
@@ -187,7 +188,8 @@ void JSActorManager::ReceiveRawMessage( 
  if (aData) {

    aData->Read(cx, &data, error);

    if (error.Failed()) {

      CHILD_DIAGNOSTIC_ASSERT(false, "Should not receive non-decodable data");

      CHILD_DIAGNOSTIC_ASSERT(CycleCollectedJSRuntime::Get()->OOMReported(),

                              "Should not receive non-decodable data");

      return;

    }

  }

xpcom/base/CycleCollectedJSRuntime.cpp

+4 −0
Original line number Diff line number Diff line
@@ -1821,6 +1821,10 @@ const char* CycleCollectedJSRuntime::OOMStateToString( 
  }

}



bool CycleCollectedJSRuntime::OOMReported() {

  return mOutOfMemoryState == OOMState::Reported;

}



void CycleCollectedJSRuntime::AnnotateAndSetOutOfMemory(OOMState* aStatePtr,

                                                        OOMState aNewState) {

  *aStatePtr = aNewState;

xpcom/base/CycleCollectedJSRuntime.h

+4 −0
Original line number Diff line number Diff line
@@ -344,6 +344,10 @@ class CycleCollectedJSRuntime { 


  const char* OOMStateToString(const OOMState aOomState) const;



  // Returns true if OOM was reported and a new successful GC cycle hasn't

  // occurred since.

  bool OOMReported();



  void SetLargeAllocationFailure(OOMState aNewState);



  void AnnotateAndSetOutOfMemory(OOMState* aStatePtr, OOMState aNewState);