bug 597736 - fixing TreeFragment leak. r=gal
authorIgor Bukanov <igor@mir2.org>
Tue, 21 Sep 2010 14:58:19 +0200
changeset 54718 b079aae532120484388644310c166dc7e2a15788
parent 54717 d0a2aec8dcb8798cdd50d09a0456c8fb77c41f09
child 54719 1bbc0fc1074723a8244af66784eb0bff0e922161
push id16011
push userrsayre@mozilla.com
push dateWed, 29 Sep 2010 06:01:57 +0000
treeherdermozilla-central@d7e659b4f80c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgal
bugs597736
milestone2.0b7pre
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 597736 - fixing TreeFragment leak. r=gal
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jscntxt.cpp
js/src/jscntxt.h
js/src/jsgc.cpp
js/src/jsscope.h
js/src/jstracer.cpp
js/src/jstracer.h
js/src/shell/js.cpp
js/src/trace-test/tests/basic/testBug597736.js
js/src/xpconnect/src/nsXPConnect.cpp
js/src/xpconnect/src/xpcprivate.h
xpcom/base/nsCycleCollector.cpp
xpcom/base/nsCycleCollector.h
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -757,22 +757,16 @@ JS_NewRuntime(uint32 maxbytes)
         JS_DestroyRuntime(rt);
         return NULL;
     }
 
     return rt;
 }
 
 JS_PUBLIC_API(void)
-JS_CommenceRuntimeShutDown(JSRuntime *rt)
-{
-    rt->gcFlushCodeCaches = true;
-}
-
-JS_PUBLIC_API(void)
 JS_DestroyRuntime(JSRuntime *rt)
 {
     rt->~JSRuntime();
 
     js_free(rt);
 }
 
 #ifdef JS_REPRMETER
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -712,18 +712,18 @@ JS_SameValue(JSContext *cx, jsval v1, js
 #define JS_NewRuntime       JS_Init
 #define JS_DestroyRuntime   JS_Finish
 #define JS_LockRuntime      JS_Lock
 #define JS_UnlockRuntime    JS_Unlock
 
 extern JS_PUBLIC_API(JSRuntime *)
 JS_NewRuntime(uint32 maxbytes);
 
-extern JS_PUBLIC_API(void)
-JS_CommenceRuntimeShutDown(JSRuntime *rt);
+/* Deprecated. */
+#define JS_CommenceRuntimeShutDown(rt) ((void) 0) 
 
 extern JS_PUBLIC_API(void)
 JS_DestroyRuntime(JSRuntime *rt);
 
 extern JS_PUBLIC_API(void)
 JS_ShutDown(void);
 
 JS_PUBLIC_API(void *)
--- a/js/src/jscntxt.cpp
+++ b/js/src/jscntxt.cpp
@@ -524,19 +524,16 @@ JSThreadData::finish()
 #endif
     stackSpace.finish();
 }
 
 void
 JSThreadData::mark(JSTracer *trc)
 {
     stackSpace.mark(trc);
-#ifdef JS_TRACER
-    traceMonitor.mark(trc);
-#endif
 }
 
 void
 JSThreadData::purge(JSContext *cx)
 {
     js_PurgeGSNCache(&gsnCache);
 
     /* FIXME: bug 506341. */
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -1026,36 +1026,40 @@ struct TraceMonitor {
     /*
      * profAlloc has a lifetime which spans exactly from js_InitJIT to
      * js_FinishJIT.
      */
     VMAllocator*            profAlloc;
     FragStatsMap*           profTab;
 #endif
 
+    bool ontrace() const {
+        return !!tracecx;
+    }
+
     /* Flush the JIT cache. */
     void flush();
 
-    /* Mark all objects baked into native code in the code cache. */
-    void mark(JSTracer *trc);
+    /* Sweep any cache entry pointing to dead GC things. */
+    void sweep();
 
     bool outOfMemory() const;
 };
 
 } /* namespace js */
 
 /*
  * N.B. JS_ON_TRACE(cx) is true if JIT code is on the stack in the current
  * thread, regardless of whether cx is the context in which that trace is
  * executing.  cx must be a context on the current thread.
  */
 #ifdef JS_TRACER
-# define JS_ON_TRACE(cx)            (JS_TRACE_MONITOR(cx).tracecx != NULL)
+# define JS_ON_TRACE(cx)            (JS_TRACE_MONITOR(cx).ontrace())
 #else
-# define JS_ON_TRACE(cx)            JS_FALSE
+# define JS_ON_TRACE(cx)            false
 #endif
 
 /* Number of potentially reusable scriptsToGC to search for the eval cache. */
 #ifndef JS_EVAL_CACHE_SHIFT
 # define JS_EVAL_CACHE_SHIFT        6
 #endif
 #define JS_EVAL_CACHE_SIZE          JS_BIT(JS_EVAL_CACHE_SHIFT)
 
@@ -1338,17 +1342,16 @@ struct JSRuntime {
     size_t              gcMaxMallocBytes;
     size_t              gcNewArenaTriggerBytes;
     uint32              gcEmptyArenaPoolLifespan;
     uint32              gcNumber;
     js::GCMarker        *gcMarkingTracer;
     uint32              gcTriggerFactor;
     size_t              gcTriggerBytes;
     volatile JSBool     gcIsNeeded;
-    volatile JSBool     gcFlushCodeCaches;
 
     /*
      * NB: do not pack another flag here by claiming gcPadding unless the new
      * flag is written only by the GC thread.  Atomic updates to packed bytes
      * are not guaranteed, so stores issued by one thread may be lost due to
      * unsynchronized read-modify-write cycles on other threads.
      */
     bool                gcPoke;
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2327,16 +2327,21 @@ MarkAndSweep(JSContext *cx, JSGCInvocati
     /* Finalize watch points associated with unreachable objects. */
     js_SweepWatchPoints(cx);
 
 #ifdef DEBUG
     /* Save the pre-sweep count of scope-mapped properties. */
     rt->liveObjectPropsPreSweep = rt->liveObjectProps;
 #endif
 
+#ifdef JS_TRACER
+    for (ThreadDataIter i(rt); !i.empty(); i.popFront())
+        i.threadData()->traceMonitor.sweep();
+#endif
+
 #ifdef JS_METHODJIT
     /* Fix-up call ICs guarding against unreachable objects. */
     mjit::SweepCallICs(cx);
 #endif
 
     /*
      * We finalize iterators before other objects so the iterator can use the
      * object which properties it enumerates over to finalize the enumeration
--- a/js/src/jsscope.h
+++ b/js/src/jsscope.h
@@ -286,16 +286,17 @@ CastAsPropertyOp(js::Class *clasp)
     return JS_DATA_TO_FUNC_PTR(PropertyOp, clasp);
 }
 
 struct Shape : public JSObjectMap
 {
     friend struct ::JSObject;
     friend struct ::JSFunction;
     friend class js::PropertyTree;
+    friend bool HasUnreachableGCThings(TreeFragment *f);
 
   protected:
     mutable js::PropertyTable *table;
 
   public:
     inline void freeTable(JSContext *cx);
 
     static bool initRuntimeState(JSContext *cx);
--- a/js/src/jstracer.cpp
+++ b/js/src/jstracer.cpp
@@ -2293,17 +2293,17 @@ ResetJIT(JSContext* cx, TraceVisFlushRea
 
 void
 FlushJITCache(JSContext *cx)
 {
     ResetJIT(cx, FR_OOM);
 }
 
 static void
-TrashTree(JSContext* cx, TreeFragment* f);
+TrashTree(TreeFragment* f);
 
 template <class T>
 static T&
 InitConst(const T &t)
 {
     return const_cast<T &>(t);
 }
 
@@ -2532,20 +2532,20 @@ TraceRecorder::TraceRecorder(JSContext* 
 }
 
 TraceRecorder::~TraceRecorder()
 {
     /* Should already have been adjusted by callers before calling delete. */
     JS_ASSERT(traceMonitor->recorder != this);
 
     if (trashSelf)
-        TrashTree(cx, fragment->root);
+        TrashTree(fragment->root);
 
     for (unsigned int i = 0; i < whichTreesToTrash.length(); i++)
-        TrashTree(cx, whichTreesToTrash[i]);
+        TrashTree(whichTreesToTrash[i]);
 
     /* Purge the tempAlloc used during recording. */
     tempAlloc().reset();
 
     forgetGuardedShapes();
 }
 
 inline bool
@@ -2610,17 +2610,17 @@ TraceRecorder::finishAbort(const char* r
      * tree. Otherwise, remove the VMSideExits we added while recording, which
      * are about to be invalid.
      *
      * BIG FAT WARNING: resetting the length is only a valid strategy as long as
      * there may be only one recorder active for a single TreeInfo at a time.
      * Otherwise, we may be throwing away another recorder's valid side exits.
      */
     if (fragment->root == fragment) {
-        TrashTree(cx, fragment->toTreeFragment());
+        TrashTree(fragment->toTreeFragment());
     } else {
         JS_ASSERT(numSideExitsBefore <= fragment->root->sideExits.length());
         fragment->root->sideExits.setLength(numSideExitsBefore);
     }
 
     /* Grab local copies of members needed after |delete this|. */
     JSContext* localcx = cx;
     TraceMonitor* localtm = traceMonitor;
@@ -2916,56 +2916,77 @@ TraceMonitor::flush()
     verbose_only( branches = NULL; )
 
     PodArrayZero(vmfragments);
     reFragments = new (alloc) REHashMap(alloc);
 
     needFlush = JS_FALSE;
 }
 
-static inline void
-MarkTree(JSTracer* trc, TreeFragment *f)
-{
+inline bool
+HasUnreachableGCThings(TreeFragment *f)
+{
+    /*
+     * We do not check here for dead scripts as JSScript is not a GC thing.
+     * Instead PurgeScriptFragments is used to remove dead script fragments.
+     * See bug 584860.
+     */
+    if (IsAboutToBeFinalized(f->globalObj))
+        return true;
     Value* vp = f->gcthings.data();
-    unsigned len = f->gcthings.length();
-    while (len--) {
+    for (unsigned len = f->gcthings.length(); len; --len) {
         Value &v = *vp++;
-        JS_SET_TRACING_NAME(trc, "jitgcthing");
         JS_ASSERT(v.isMarkable());
-        MarkGCThing(trc, v.toGCThing(), v.gcKind());
+        if (IsAboutToBeFinalized(v.toGCThing()))
+            return true;
     }
     const Shape** shapep = f->shapes.data();
-    len = f->shapes.length();
-    while (len--) {
+    for (unsigned len = f->shapes.length(); len; --len) {
         const Shape* shape = *shapep++;
-        shape->trace(trc);
-    }
+        if (!shape->marked())
+            return true;
+    }
+    return false;
 }
 
 void
-TraceMonitor::mark(JSTracer* trc)
-{
-    if (!trc->context->runtime->gcFlushCodeCaches) {
-        for (size_t i = 0; i < FRAGMENT_TABLE_SIZE; ++i) {
-            TreeFragment* f = vmfragments[i];
-            while (f) {
-                if (f->code())
-                    MarkTree(trc, f);
-                TreeFragment* peer = f->peer;
-                while (peer) {
-                    if (peer->code())
-                        MarkTree(trc, peer);
-                    peer = peer->peer;
-                }
-                f = f->next;
+TraceMonitor::sweep()
+{
+    JS_ASSERT(!ontrace());
+    debug_only_print0(LC_TMTracer, "Purging fragments with dead things");
+
+    for (size_t i = 0; i < FRAGMENT_TABLE_SIZE; ++i) {
+        TreeFragment** fragp = &vmfragments[i];
+        while (TreeFragment* frag = *fragp) {
+            TreeFragment* peer = frag;
+            do {
+                if (HasUnreachableGCThings(peer))
+                    break;
+                peer = peer->peer;
+            } while (peer);
+            if (peer) {
+                debug_only_printf(LC_TMTracer,
+                                  "TreeFragment peer %p has dead gc thing."
+                                  "Disconnecting tree %p with ip %p\n",
+                                  (void *) peer, (void *) frag, frag->ip);
+                JS_ASSERT(frag->root == frag);
+                *fragp = frag->next;
+                do {
+                    verbose_only( FragProfiling_FragFinalizer(frag, this); )
+                    TrashTree(frag);
+                    frag = frag->peer;
+                } while (frag);
+            } else {
+                fragp = &frag->next;
             }
         }
-        if (recorder)
-            MarkTree(trc, recorder->getTree());
-    }
+    }
+
+    if (recorder && HasUnreachableGCThings(recorder->getTree()))
+        recorder->finishAbort("dead GC things");
 }
 
 /*
  * Box a value from the native stack back into the Value format.
  */
 static inline void
 NativeToValue(JSContext* cx, Value& v, JSValueType type, double* slot)
 {
@@ -5667,34 +5688,34 @@ TraceRecorder::startRecorder(JSContext* 
         ResetJIT(cx, FR_OOM);
         return false;
     }
 
     return true;
 }
 
 static void
-TrashTree(JSContext* cx, TreeFragment* f)
+TrashTree(TreeFragment* f)
 {
     JS_ASSERT(f == f->root);
     debug_only_printf(LC_TMTreeVis, "TREEVIS TRASH FRAG=%p\n", (void*)f);
 
     if (!f->code())
         return;
     AUDIT(treesTrashed);
     debug_only_print0(LC_TMTracer, "Trashing tree info.\n");
     f->setCode(NULL);
     TreeFragment** data = f->dependentTrees.data();
     unsigned length = f->dependentTrees.length();
     for (unsigned n = 0; n < length; ++n)
-        TrashTree(cx, data[n]);
+        TrashTree(data[n]);
     data = f->linkedTrees.data();
     length = f->linkedTrees.length();
     for (unsigned n = 0; n < length; ++n)
-        TrashTree(cx, data[n]);
+        TrashTree(data[n]);
 }
 
 static void
 SynthesizeFrame(JSContext* cx, const FrameInfo& fi, JSObject* callee)
 {
     VOUCH_DOES_NOT_REQUIRE_STACK();
 
     /* Assert that we have a correct sp distance from cx->fp()->slots in fi. */
@@ -5896,17 +5917,17 @@ AttemptToStabilizeTree(JSContext* cx, JS
         JS_ASSERT(from->nGlobalTypes() == from->globalSlots->length());
         /* This exit is no longer unstable, so remove it. */
         if (exit->exitType == UNSTABLE_LOOP_EXIT)
             from->removeUnstableExit(exit);
         debug_only_stmt(DumpPeerStability(tm, peer->ip, globalObj, from->globalShape, from->argc);)
         return false;
     } else if (consensus == TypeConsensus_Undemotes) {
         /* The original tree is unconnectable, so trash it. */
-        TrashTree(cx, peer);
+        TrashTree(peer);
         return false;
     }
 
     SlotList *globalSlots = from->globalSlots;
 
     JS_ASSERT(from == from->root);
 
     /* If this tree has been blacklisted, don't try to record a new one. */
@@ -7808,32 +7829,37 @@ FinishJIT(TraceMonitor *tm)
 
 JS_REQUIRES_STACK void
 PurgeScriptFragments(JSContext* cx, JSScript* script)
 {
     debug_only_printf(LC_TMTracer,
                       "Purging fragments for JSScript %p.\n", (void*)script);
 
     TraceMonitor* tm = &JS_TRACE_MONITOR(cx);
+
+    /* A recorder script is being evaluated and can not be destroyed or GC-ed. */
+    JS_ASSERT_IF(tm->recorder, 
+                 JS_UPTRDIFF(tm->recorder->getTree()->ip, script->code) >= script->length);
+
     for (size_t i = 0; i < FRAGMENT_TABLE_SIZE; ++i) {
         TreeFragment** fragp = &tm->vmfragments[i];
         while (TreeFragment* frag = *fragp) {
             if (JS_UPTRDIFF(frag->ip, script->code) < script->length) {
                 /* This fragment is associated with the script. */
                 debug_only_printf(LC_TMTracer,
                                   "Disconnecting TreeFragment %p "
                                   "with ip %p, in range [%p,%p).\n",
                                   (void*)frag, frag->ip, script->code,
                                   script->code + script->length);
 
                 JS_ASSERT(frag->root == frag);
                 *fragp = frag->next;
                 do {
                     verbose_only( FragProfiling_FragFinalizer(frag, tm); )
-                    TrashTree(cx, frag);
+                    TrashTree(frag);
                 } while ((frag = frag->peer) != NULL);
                 continue;
             }
             fragp = &frag->next;
         }
     }
 
     RecordAttemptMap &table = *tm->recordAttempts;
--- a/js/src/jstracer.h
+++ b/js/src/jstracer.h
@@ -1419,18 +1419,19 @@ class TraceRecorder
     friend class DetermineTypesVisitor;
     friend class RecursiveSlotMap;
     friend class UpRecursiveSlotMap;
     friend MonitorResult MonitorLoopEdge(JSContext*, uintN&);
     friend TracePointAction MonitorTracePoint(JSContext*, uintN &inlineCallCount,
                                               bool &blacklist);
     friend void AbortRecording(JSContext*, const char*);
     friend class BoxArg;
+    friend void TraceMonitor::sweep();
 
-public:
+  public:
     static bool JS_REQUIRES_STACK
     startRecorder(JSContext*, VMSideExit*, VMFragment*,
                   unsigned stackSlots, unsigned ngslots, JSValueType* typeMap,
                   VMSideExit* expectedInnerExit, jsbytecode* outerTree,
                   uint32 outerArgc, bool speculate);
 
     /* Accessors. */
     VMFragment*         getFragment() const { return fragment; }
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -5343,18 +5343,16 @@ main(int argc, char **argv, char **envp)
     cx = NewContext(rt);
     if (!cx)
         return 1;
 
     JS_SetGCParameterForThread(cx, JSGC_MAX_CODE_CACHE_BYTES, 16 * 1024 * 1024);
 
     result = shell(cx, argc, argv, envp);
 
-    JS_CommenceRuntimeShutDown(rt);
-
     DestroyContext(cx, true);
 
     KillWatchdog();
 
     JS_DestroyRuntime(rt);
     JS_ShutDown();
     return result;
 }
new file mode 100644
--- /dev/null
+++ b/js/src/trace-test/tests/basic/testBug597736.js
@@ -0,0 +1,32 @@
+function leak_test() {
+    // Create a reference loop function->script->traceFragment->object->function
+    // that GC must be able to break. To embedd object into the fragment the
+    // code use prototype chain of depth 2 which caches obj.__proto__.__proto__
+    // into the fragment.
+
+    // To make sure that we have no references to the function f after this
+    // function returns due via the conservative scan of the native stack we
+    // loop here twice overwriting the stack and registers with new garabge.
+    for (var j = 0; j != 2; ++j) {
+	var f = Function("a", "var s = 0; for (var i = 0; i != 100; ++i) s += a.b; return s;");
+	var c = {b: 1, f: f, leakDetection: makeFinalizeObserver()};
+	f({ __proto__: { __proto__: c}});
+	f = c = a = null;
+	gc();
+    }
+}
+
+function test()
+{
+    if (typeof finalizeCount != "function")
+	return;
+
+    var base = finalizeCount();
+    leak_test();
+    gc();
+    gc();
+    var n = finalizeCount();
+    assertEq(base < finalizeCount(), true, "Some finalizations must happen");
+}
+
+test();
--- a/js/src/xpconnect/src/nsXPConnect.cpp
+++ b/js/src/xpconnect/src/nsXPConnect.cpp
@@ -507,27 +507,16 @@ nsXPConnect::FinishCycleCollection()
 nsCycleCollectionParticipant *
 nsXPConnect::ToParticipant(void *p)
 {
     if (!ADD_TO_CC(js_GetGCThingTraceKind(p)))
         return NULL;
     return this;
 }
 
-void
-nsXPConnect::CommenceShutdown()
-{
-#ifdef DEBUG
-    fprintf(stderr, "nsXPConnect::CommenceShutdown()\n");
-#endif
-    // Tell the JS engine that we are about to destroy the runtime.
-    JSRuntime* rt = mRuntime->GetJSRuntime();
-    JS_CommenceRuntimeShutDown(rt);
-}
-
 NS_IMETHODIMP
 nsXPConnect::RootAndUnlinkJSObjects(void *p)
 {
     return NS_OK;
 }
 
 #ifdef DEBUG_CC
 void
--- a/js/src/xpconnect/src/xpcprivate.h
+++ b/js/src/xpconnect/src/xpcprivate.h
@@ -491,17 +491,16 @@ public:
     NS_IMETHOD Traverse(void *p,
                         nsCycleCollectionTraversalCallback &cb);
     
     // nsCycleCollectionLanguageRuntime
     virtual nsresult BeginCycleCollection(nsCycleCollectionTraversalCallback &cb,
                                           bool explainExpectedLiveGarbage);
     virtual nsresult FinishCycleCollection();
     virtual nsCycleCollectionParticipant *ToParticipant(void *p);
-    virtual void CommenceShutdown();
     virtual void Collect();
 #ifdef DEBUG_CC
     virtual void PrintAllReferencesTo(void *p);
 #endif
 
     unsigned GetOutstandingRequests(JSContext* cx);
 
     // This returns the singleton nsCycleCollectionParticipant for JSContexts.
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -936,20 +936,16 @@ struct nsCycleCollectionXPCOMRuntime :
 
     nsresult FinishCycleCollection() 
     {
         return NS_OK;
     }
 
     inline nsCycleCollectionParticipant *ToParticipant(void *p);
 
-    void CommenceShutdown()
-    {
-    }
-
 #ifdef DEBUG_CC
     virtual void PrintAllReferencesTo(void *p) {}
 #endif
 };
 
 struct nsCycleCollector
 {
     PRBool mCollectionInProgress;
@@ -2722,21 +2718,16 @@ nsCycleCollector::SuspectedCount()
 }
 
 void
 nsCycleCollector::Shutdown()
 {
     // Here we want to run a final collection and then permanently
     // disable the collector because the program is shutting down.
 
-    for (PRUint32 i = 0; i <= nsIProgrammingLanguage::MAX; ++i) {
-        if (mRuntimes[i])
-            mRuntimes[i]->CommenceShutdown();
-    }
-
     Collect(SHUTDOWN_COLLECTIONS(mParams), nsnull);
 
 #ifdef DEBUG_CC
     GCGraphBuilder builder(mGraph, mRuntimes, nsnull);
     mScanInProgress = PR_TRUE;
     SelectPurple(builder);
     mScanInProgress = PR_FALSE;
     if (builder.Count() != 0) {
--- a/xpcom/base/nsCycleCollector.h
+++ b/xpcom/base/nsCycleCollector.h
@@ -51,17 +51,16 @@ class nsCycleCollectionTraversalCallback
 // implements language-specific aspects of the cycle collection task.
 
 struct nsCycleCollectionLanguageRuntime
 {
     virtual nsresult BeginCycleCollection(nsCycleCollectionTraversalCallback &cb,
                                           bool explainLiveExpectedGarbage) = 0;
     virtual nsresult FinishCycleCollection() = 0;
     virtual nsCycleCollectionParticipant *ToParticipant(void *p) = 0;
-    virtual void CommenceShutdown() = 0;
 #ifdef DEBUG_CC
     virtual void PrintAllReferencesTo(void *p) = 0;
 #endif
 };
 
 nsresult nsCycleCollector_startup();
 // Returns the number of collected nodes.
 NS_COM PRUint32 nsCycleCollector_collect(nsICycleCollectorListener *aListener);