Bug 1587638: Clear GC pointers in ScriptSource when canonical ScriptSourceObject is finalized r=tcampbell
authorIain Ireland <iireland@mozilla.com>
Thu, 31 Oct 2019 21:10:41 +0000
changeset 500396 9d297847a45226ba0a642510e547cf5952d7a991
parent 500395 6c358363b281222f619014f75e734846ad7f41c6
child 500397 8834ca679ef71dfa07a0b6842c07d895027c1893
push id36763
push userrmaries@mozilla.com
push dateMon, 04 Nov 2019 21:44:06 +0000
treeherdermozilla-central@75a7a3400888 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1587638
milestone72.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 1587638: Clear GC pointers in ScriptSource when canonical ScriptSourceObject is finalized r=tcampbell Right now there BinAST is the only case, but subsequent patches will add GC pointers to XDR encoders and decoders. Differential Revision: https://phabricator.services.mozilla.com/D48781
js/src/vm/JSScript.cpp
js/src/vm/JSScript.h
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -1616,16 +1616,19 @@ void JSScript::resetScriptCounts() {
   for (PCCounts& elem : sc.throwCounts_) {
     elem.numExec() = 0;
   }
 }
 
 void ScriptSourceObject::finalize(JSFreeOp* fop, JSObject* obj) {
   MOZ_ASSERT(fop->onMainThread());
   ScriptSourceObject* sso = &obj->as<ScriptSourceObject>();
+  if (sso->isCanonical()) {
+    sso->source()->finalizeGCData();
+  }
   sso->source()->decref();
 
   // Clear the private value, calling the release hook if necessary.
   sso->setPrivate(fop->runtime(), UndefinedValue());
 }
 
 void ScriptSourceObject::trace(JSTracer* trc, JSObject* obj) {
   // This can be invoked during allocation of the SSO itself, before we've had a
@@ -2518,27 +2521,56 @@ bool ScriptSource::assignSource(JSContex
 template bool ScriptSource::assignSource(JSContext* cx,
                                          const ReadOnlyCompileOptions& options,
                                          SourceText<char16_t>& srcBuf);
 template bool ScriptSource::assignSource(JSContext* cx,
                                          const ReadOnlyCompileOptions& options,
                                          SourceText<Utf8Unit>& srcBuf);
 
 void ScriptSource::trace(JSTracer* trc) {
+  // This should be kept in sync with ScriptSource::finalizeGCData below.
 #ifdef JS_BUILD_BINAST
   if (data.is<BinAST>()) {
     if (auto& metadata = data.as<BinAST>().metadata) {
       metadata->trace(trc);
     }
   }
 #else
   MOZ_ASSERT(!data.is<BinAST>());
 #endif  // JS_BUILD_BINAST
 }
 
+void ScriptSource::finalizeGCData() {
+  // This should be kept in sync with ScriptSource::trace above.
+
+  // When the canonical ScriptSourceObject's finalizer runs, this
+  // ScriptSource can no longer be accessed from the main
+  // thread. However, an offthread source compression task may still
+  // hold a reference. We must clean up any GC pointers owned by this
+  // ScriptSource now, because trying to run those prebarriers
+  // offthread later will fail.
+  MOZ_ASSERT(TlsContext.get() && TlsContext.get()->isMainThreadContext());
+
+#ifdef JS_BUILD_BINAST
+  if (hasBinASTSource()) {
+    if (auto& metadata = data.as<BinAST>().metadata) {
+      metadata.reset();
+    }
+  }
+#endif  // JS_BUILD_BINAST
+}
+
+ScriptSource::~ScriptSource() {
+  MOZ_ASSERT(refs == 0);
+
+  // GC pointers must have been cleared earlier, because this destructor could
+  // be called off-thread by SweepCompressionTasks. See above.
+  MOZ_ASSERT_IF(hasBinASTSource(), !data.as<BinAST>().metadata);
+}
+
 static MOZ_MUST_USE bool reallocUniquePtr(UniqueChars& unique, size_t size) {
   auto newPtr = static_cast<char*>(js_realloc(unique.get(), size));
   if (!newPtr) {
     return false;
   }
 
   // Since the realloc succeeded, unique is now holding a freed pointer.
   mozilla::Unused << unique.release();
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -724,17 +724,18 @@ class ScriptSource {
 
  public:
   // When creating a JSString* from TwoByte source characters, we don't try to
   // to deflate to Latin1 for longer strings, because this can be slow.
   static const size_t SourceDeflateLimit = 100;
 
   explicit ScriptSource() : id_(++idCount_) {}
 
-  ~ScriptSource() { MOZ_ASSERT(refs == 0); }
+  void finalizeGCData();
+  ~ScriptSource();
 
   void incref() { refs++; }
   void decref() {
     MOZ_ASSERT(refs != 0);
     if (--refs == 0) {
       js_delete(this);
     }
   }