Bug 1364528 - Don't synchronously finalize native objects if an exception is pending. r=smaug
authorAndrew McCreight <continuation@gmail.com>
Fri, 12 May 2017 11:16:16 -0700
changeset 358152 a61ed4643a773ad9354bec5dbb0aea14e1ec8756
parent 358151 8a388bbc15120dc8a96c9a2010e1f3286127e290
child 358153 6cd198caa3e609d14b073ec1d79ad1bdb425276f
push id31818
push userarchaeopteryx@coole-files.de
push dateSun, 14 May 2017 16:01:21 +0000
treeherdermozilla-central@3a801856dea9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1364528
milestone55.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 1364528 - Don't synchronously finalize native objects if an exception is pending. r=smaug MozReview-Commit-ID: 6OY3ftH1aWu
xpcom/base/CycleCollectedJSRuntime.cpp
xpcom/base/CycleCollectedJSRuntime.h
--- a/xpcom/base/CycleCollectedJSRuntime.cpp
+++ b/xpcom/base/CycleCollectedJSRuntime.cpp
@@ -814,17 +814,17 @@ CycleCollectedJSRuntime::GCCallback(JSCo
                                     JSGCStatus aStatus,
                                     void* aData)
 {
   CycleCollectedJSRuntime* self = static_cast<CycleCollectedJSRuntime*>(aData);
 
   MOZ_ASSERT(CycleCollectedJSContext::Get()->Context() == aContext);
   MOZ_ASSERT(CycleCollectedJSContext::Get()->Runtime() == self);
 
-  self->OnGC(aStatus);
+  self->OnGC(aContext, aStatus);
 }
 
 /* static */ void
 CycleCollectedJSRuntime::GCSliceCallback(JSContext* aContext,
                                          JS::GCProgress aProgress,
                                          const JS::GCDescription& aDesc)
 {
   CycleCollectedJSRuntime* self = CycleCollectedJSRuntime::Get();
@@ -1418,37 +1418,47 @@ CycleCollectedJSRuntime::AnnotateAndSetO
                                      ? NS_LITERAL_CSTRING("Reporting")
                                      : aNewState == OOMState::Reported
                                      ? NS_LITERAL_CSTRING("Reported")
                                      : NS_LITERAL_CSTRING("Recovered"));
 #endif
 }
 
 void
-CycleCollectedJSRuntime::OnGC(JSGCStatus aStatus)
+CycleCollectedJSRuntime::OnGC(JSContext* aContext,
+                              JSGCStatus aStatus)
 {
   switch (aStatus) {
     case JSGC_BEGIN:
       nsCycleCollector_prepareForGarbageCollection();
       mZonesWaitingForGC.Clear();
       break;
     case JSGC_END: {
 #ifdef MOZ_CRASHREPORTER
       if (mOutOfMemoryState == OOMState::Reported) {
         AnnotateAndSetOutOfMemory(&mOutOfMemoryState, OOMState::Recovered);
       }
       if (mLargeAllocationFailureState == OOMState::Reported) {
         AnnotateAndSetOutOfMemory(&mLargeAllocationFailureState, OOMState::Recovered);
       }
 #endif
 
-      // Do any deferred finalization of native objects.
-      FinalizeDeferredThings(JS::WasIncrementalGC(mJSRuntime)
+      // Do any deferred finalization of native objects. Normally we do this
+      // incrementally for an incremental GC, and immediately for a
+      // non-incremental GC, on the basis that the type of GC reflects how
+      // urgently resources should be destroyed. However under some circumstances
+      // (such as in js::InternalCallOrConstruct) we can end up running a
+      // non-incremental GC when there is a pending exception, and the finalizers
+      // are not set up to handle that. In that case, just run them later, after
+      // we've returned to the event loop.
+      bool finalizeIncrementally = JS::WasIncrementalGC(mJSRuntime) || JS_IsExceptionPending(aContext);
+      FinalizeDeferredThings(finalizeIncrementally
                              ? CycleCollectedJSContext::FinalizeIncrementally
                              : CycleCollectedJSContext::FinalizeNow);
+
       break;
     }
     default:
       MOZ_CRASH();
   }
 
   CustomGCCallback(aStatus);
 }
--- a/xpcom/base/CycleCollectedJSRuntime.h
+++ b/xpcom/base/CycleCollectedJSRuntime.h
@@ -231,17 +231,17 @@ public:
     // GC is taken as a proxy for "we've been banging on the heap a good bit
     // now and haven't crashed; the OOM was probably handled correctly".
     Recovered
   };
 
   void SetLargeAllocationFailure(OOMState aNewState);
 
   void AnnotateAndSetOutOfMemory(OOMState* aStatePtr, OOMState aNewState);
-  void OnGC(JSGCStatus aStatus);
+  void OnGC(JSContext* aContext, JSGCStatus aStatus);
   void OnOutOfMemory();
   void OnLargeAllocationFailure();
 
   JSRuntime* Runtime() { return mJSRuntime; }
 
 public:
   void AddJSHolder(void* aHolder, nsScriptObjectTracer* aTracer);
   void RemoveJSHolder(void* aHolder);