341675 - Iterators: still infinite loop during GC

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

Consider the following example for jsshell:

var globalToPokeGC = {};

function make_iterator()
{
	var iter = (function() { yield 0; })();
	iter.close = make_iterator;
	globalToPokeGC = {};
}

make_iterator();
gc();

This will loop infinitely during GC since make_iterator when executed from a close hook for iter object will set JSRuntime.gcPoke. That triggers GC restart.

Comment 1

[Igor Bukanov]

Attachment: Fix

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 225772 [details] [diff] [review]
Fix

>--- .pc/fix341675.diff/js/src/jsgc.c	2006-06-15 22:24:15.000000000 +0200
>+++ js/src/jsgc.c	2006-06-15 23:29:17.000000000 +0200
>@@ -2176,16 +2176,17 @@ js_GC(JSContext *cx, uintN gcflags)
>     JSObjectsToClose objectsToClose;
>     size_t nbytes, limit, offset;
>     JSGCArena *a, **ap;
>     uint8 flags, *flagp, *firstPage;
>     JSGCThing *thing, *freeList;
>     JSGCArenaList *arenaList;
>     GCFinalizeOp finalizer;
>     JSBool allClear;
>+    JSBool shouldRestart;

Feel free to combine these JSBool declarations to share the same type and line.

>     /*
>+     * We want to restart GC if any of the finalizers called js_RemoveRoot
>+     * js_UnlockGCThingRT.

Missing "or" before "js_UnlockGCThingRT".

>+         * On the last context

destroy context

> restart GC to collect just closed objects. This
>+         * does not cause infinite loops with close hooks creating more
>+         * closable

closeable

> objects since we do not allow to install close hooks during
>+         * the shutdown of runtime. See bug 340889 and bug 341675.

More comment nits, still vacationing (sort of).

/be

Comment 3

[Igor Bukanov]

Attachment: Fix v1b

Here is patch's version to address the nits.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 225861 [details] [diff] [review]
Fix v1b

>+         * creating more closeable objects since we do not allow to install

"installing", perhaps?

Comment 5

[Igor Bukanov]

Attachment: Fix v1c

Patch to commit with the comments improvment.

Comment 6

[Igor Bukanov]

I committed to the trunk the patch from comment 5. 

Comment 7

[Bob Clary [:bc] (inactive)]

RCS file: /cvsroot/mozilla/js/tests/js1_7/GC/regress-341675.js,v
done
Checking in regress-341675.js;
/cvsroot/mozilla/js/tests/js1_7/GC/regress-341675.js,v  <--  regress-341675.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/js/tests/js1_7/GC/shell.js,v
done
Checking in shell.js;
/cvsroot/mozilla/js/tests/js1_7/GC/shell.js,v  <--  shell.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/js/tests/js1_7/GC/browser.js,v
done
Checking in browser.js;
/cvsroot/mozilla/js/tests/js1_7/GC/browser.js,v  <--  browser.js
initial revision: 1.1
done

note to self: could not reproduce the infinite loop in 20060613 nightly or today's build on winxp.

Comment 8

[Bob Clary [:bc] (inactive)]

crash at least windows and macppc trunk browser  js1_7/GC/regress-341675.js

		maxFreeThings	0x00000006	unsigned int
+		tmpthing	0x000000aa {next=??? flagp=??? }	JSGCThing *

>	js3250.dll!js_NewGCThing(JSContext * cx=0x049a7b58, unsigned int flags=0x00000002, unsigned int nbytes=0x00000008)  Line 1027 + 0x3 bytes	C
 	js3250.dll!js_NewDouble(JSContext * cx=0x049a7b58, double d=1.#INF000000000000, unsigned int gcflag=0x00000000)  Line 644 + 0x12 bytes	C
 	js3250.dll!js_NewDoubleValue(JSContext * cx=0x049a7b58, double d=1.#INF000000000000, long * rval=0x0012f4f4)  Line 662 + 0x14 bytes	C
 	js3250.dll!js_Interpret(JSContext * cx=0x049a7b58, unsigned char * pc=0x04c51383, long * result=0x0012f710)  Line 3350 + 0x261 bytes	C
 	js3250.dll!js_Execute(JSContext * cx=0x049a7b58, JSObject * chain=0x04b24848, JSScript * script=0x04c35058, JSStackFrame * down=0x00000000, unsigned int flags=0x00000000, long * result=0x0012f83c)  Line 1573 + 0x13 bytes	C
 	js3250.dll!JS_EvaluateUCScriptForPrincipals(JSContext * cx=0x049a7b58, JSObject * obj=0x04b24848, JSPrincipals * principals=0x03c6531c, const unsigned short * chars=0x04c33e78, unsigned int length=0x000008d6, const char * filename=0x04c419f0, unsigned int lineno=0x00000001, long * rval=0x0012f83c)  Line 4293 + 0x19 bytes	C
 	gklayout.dll!nsJSContext::EvaluateString(const nsAString_internal & aScript={...}, void * aScopeObject=0x04b24848, nsIPrincipal * aPrincipal=0x03c65318, const char * aURL=0x04c419f0, unsigned int aLineNo=0x00000001, unsigned int aVersion=0x00000000, nsAString_internal * aRetValue=0x00000000, int * aIsUndefined=0x0012f930)  Line 1247 + 0x43 bytes	C++
 	gklayout.dll!nsScriptLoader::EvaluateScript(nsScriptLoadRequest * aRequest=0x04c41628, const nsString & aScript={...})  Line 800 + 0x63 bytes	C++
 	gklayout.dll!nsScriptLoader::ProcessRequest(nsScriptLoadRequest * aRequest=0x04c41628)  Line 704 + 0x13 bytes	C++
 	gklayout.dll!nsScriptLoader::OnStreamComplete(nsIStreamLoader * aLoader=0x04c418b0, nsISupports * aContext=0x04c41628, unsigned int aStatus=0x00000000, unsigned int stringLen=0x000008d6, const unsigned char * string=0x04c5c240)  Line 1065	C++
 	necko.dll!nsStreamLoader::OnStopRequest(nsIRequest * request=0x04c41ab8, nsISupports * ctxt=0x04c41628, unsigned int aStatus=0x00000000)  Line 117	C++
 	necko.dll!nsStreamListenerTee::OnStopRequest(nsIRequest * request=0x04c41ab8, nsISupports * context=0x04c41628, unsigned int status=0x00000000)  Line 66	C++
 	necko.dll!nsHttpChannel::OnStopRequest(nsIRequest * request=0x04c54780, nsISupports * ctxt=0x00000000, unsigned int status=0x00000000)  Line 4053	C++
 	necko.dll!nsInputStreamPump::OnStateStop()  Line 567	C++
 	necko.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream=0x04c54560)  Line 391 + 0xb bytes	C++
 	xpcom_core.dll!nsInputStreamReadyEvent::Run()  Line 112	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=0x00000001, int * result=0x0012fc34)  Line 483	C++
 	xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00b08e00, int mayWait=0x00000001)  Line 225 + 0x16 bytes	C++
 	gkwidget.dll!nsBaseAppShell::Run()  Line 153 + 0xc bytes	C++
 	tkitcmps.dll!nsAppStartup::Run()  Line 171 + 0x1c bytes	C++
 	xul.dll!XRE_main(int argc=0x00000004, char * * argv=0x00b08180, const nsXREAppData * aAppData=0x004036b0)  Line 2349 + 0x25 bytes	C++
 	firefox.exe!main(int argc=0x00000004, char * * argv=0x00b08180)  Line 61 + 0x13 bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 586 + 0x19 bytes	C
 	firefox.exe!mainCRTStartup()  Line 403	C
 	kernel32.dll!_BaseProcessStart@4()  + 0x23 bytes	

Comment 9

[Brendan Eich [:brendan]]

Bob, can you file a new bug on the crash you are seeing?

/be

Comment 10

[Bob Clary [:bc] (inactive)]

(In reply to comment #9)

Bug 343295

Comment 11

[Bob Clary [:bc] (inactive)]

fixed by Bug 336373 on the 1.8.1 branch. 
verified fixed 1.8.1 with windows/macppc/linux 20060707

crashes in 1.8.1 as in trunk. see bug 343295

Comment 12

[Bob Clary [:bc] (inactive)]

verified fixed 1.9 20060818 windows/mac(ppc|tel)/linux

Comment 13

[Bob Clary [:bc] (inactive)]

1.9 20060830 js1_7/GC/regress-341675.js now fail with uncaught exception: [object StopIteration] I can't seem to catch that.

Comment 14

[Igor Bukanov]

(In reply to comment #13)
> 1.9 20060830 js1_7/GC/regress-341675.js now fail with uncaught exception:
> [object StopIteration] I can't seem to catch that.

You can not catch it since it is reported by GC. But the reason for StopIteration is just filed bug 350837. What happens here is that make_iterator (see below) is called with cx->throwing set. As it is an inlined function, the bug does not affect it. When it calls iter.next later, cx->throwing is still set and is propagated to the native implementation of the generator which eventually calls js_Interpret. That quits immediately as throwing is set. It is interpreted as a generator that returns during calling "next". Thus the implementation throws StopIteration exception. It eventually is reported by the GC.

 
function generator()
{
  try {
    yield [];
  } finally {
    make_iterator();
  }
}

function make_iterator()
{
  var iter = generator();
  iter.next();
}

make_iterator();
gc();