Bug 708838: Use findReferences instead of countHeap to make js1_8/extensions/regress-422269.js fail less randomly. r=jwalden

The SpiderMonkey test js/src/tests/js1_8/extensions/regress-422269.js fails
randomly, because it requires a certain object to be garbage-collected to
pass. With a conservative stack scanner, an object being retained is not
necessarily a bug, and in general, the engine makes no promises about which
objects it retains and which it doesn't.

The JavaScript shell's new findReferences function allows us to find all GC
edges referring to the object, and filter out ones we know to be benign.
This patch changes the test to use findReferences.

js/src/shell/js.cpp

@@ -4121,7 +4121,28 @@ static const char *const shell_help_messages[] = {
 "notes([fun])             Show source notes for functions",
 "stats([string ...])      Dump 'arena', 'atom', 'global' stats",
 "findReferences(target)\n"
-"  Walk the heap and return an object describing all references to target",
+"  Walk the entire heap, looking for references to |target|, and return a\n"
+"  \"references object\" describing what we found.\n"
+"\n"
+"  Each property of the references object describes one kind of reference. The\n"
+"  property's name is the label supplied to MarkObject, JS_CALL_TRACER, or what\n"
+"  have you, prefixed with \"edge: \" to avoid collisions with system properties\n"
+"  (like \"toString\" and \"__proto__\"). The property's value is an array of things\n"
+"  that refer to |thing| via that kind of reference. Ordinary references from\n"
+"  one object to another are named after the property name (with the \"edge: \"\n"
+"  prefix).\n"
+"\n"
+"  Garbage collection roots appear as references from 'null'. We use the name\n"
+"  given to the root (with the \"edge: \" prefix) as the name of the reference.\n"
+"\n"
+"  Note that the references object does record references from objects that are\n"
+"  only reachable via |thing| itself, not just the references reachable\n"
+"  themselves from roots that keep |thing| from being collected. (We could make\n"
+"  this distinction if it is useful.)\n"
+"\n"
+"  If any references are found by the conservative scanner, the references\n"
+"  object will have a property named \"edge: machine stack\"; the referrers will\n"
+"  be 'null', because they are roots.",
 #endif
 "dumpStack()              Dump the stack as an array of callees (youngest first)",
 #ifdef TEST_CVTARGS

js/src/shell/jsheaptools.cpp

@@ -551,32 +551,7 @@ ReferenceFinder::findReferences(JSObject *target)
     return result;
 }
 
-/*
- * findReferences(thing)
- *
- * Walk the entire heap, looking for references to |thing|, and return a
- * "references object" describing what we found.
- *
- * Each property of the references object describes one kind of reference. The
- * property's name is the label supplied to MarkObject, JS_CALL_TRACER, or what
- * have you, prefixed with "edge: " to avoid collisions with system properties
- * (like "toString" and "__proto__"). The property's value is an array of things
- * that refer to |thing| via that kind of reference. Ordinary references from
- * one object to another are named after the property name (with the "edge: "
- * prefix).
- *
- * Garbage collection roots appear as references from 'null'. We use the name
- * given to the root (with the "edge: " prefix) as the name of the reference.
- *
- * Note that the references object does record references from objects that are
- * only reachable via |thing| itself, not just the references reachable
- * themselves from roots that keep |thing| from being collected. (We could make
- * this distinction if it is useful.)
- *
- * If any references are found by the conservative scanner, the references
- * object will have a property named "edge: machine stack"; the referrers will
- * be 'null', because they are roots.
- */
+/* See help(findReferences). */
 JSBool
 FindReferences(JSContext *cx, uintN argc, jsval *vp)
 {

js/src/tests/js1_8/extensions/regress-422269.js

@@ -38,8 +38,8 @@
 //-----------------------------------------------------------------------------
 var BUGNUMBER = 422269;
 var summary = 'Compile-time let block should not capture runtime references';
-var actual = 'No leak';
-var expect = 'No leak';
+var actual = 'referenced only by stack and closure';
+var expect = 'referenced only by stack and closure';
 
 
 //-----------------------------------------------------------------------------
@@ -56,31 +56,39 @@ function test()
   function f()
   {
     let m = {sin: Math.sin};
-    (function() { m.sin(1); })();
+    (function holder() { m.sin(1); })();
     return m;
   }
 
-  if (typeof countHeap == 'undefined')
+  if (typeof findReferences == 'undefined')
   {
     expect = actual = 'Test skipped';
-    print('Test skipped. Requires countHeap function.');
+    print('Test skipped. Requires findReferences function.');
   }
   else
   {
     var x = f();
-    f(); // overwrite the machine stack with new objects
-    gc();
-    var n = countHeap();
-    x = null;
-    // When running with the method JIT, null may not get stored to memory right away.
-    // Calling eval ensures that all values are stored out so that the old x is no
-    // longer rooted from the stack.
-    eval("");
-    gc();
+    var refs = findReferences(x);
 
-    var n2 = countHeap();
-    if (n2 >= n)
-      actual = "leak is detected, something roots the result of f";
+    // At this point, x should only be referenced from the stack --- the
+    // variable 'x' itself, and any random things the conservative scanner
+    // finds --- and possibly from the 'holder' closure, which could itself
+    // be held alive for random reasons. Remove those from the refs list, and 
+    // then complain if anything is left.
+    for (var edge in refs) {
+      // Remove references from roots, like the stack.
+      if (refs[edge].every(function (r) r === null))
+        delete refs[edge];
+      // Remove references from the closure, which could be held alive for
+      // random reasons.
+      else if (refs[edge].length === 1 &&
+               typeof refs[edge][0] === "function" &&
+               refs[edge][0].name === "holder")
+        delete refs[edge];
+    }
+
+    if (Object.keys(refs).length != 0)
+        actual = "unexpected references to the result of f: " + Object.keys(refs).join(", ");
   }
   reportCompare(expect, actual, summary);