600310 - TM: don't perform GC outside of stack quota

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gregor Wagner [:gwagner]]

from Bug 600139

Comment 1

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 2

[Gregor Wagner [:gwagner]]

Attachment: patch

Assert in js_GC

Comment 3

[Gregor Wagner [:gwagner]]

Comment on attachment 480326 [details] [diff] [review]
patch

No it's not that easy...

Comment 4

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 5

[Igor Bukanov]

Comment on attachment 480359 [details] [diff] [review]
patch

>diff -r eb83de9d08da js/src/jsgc.cpp
>--- a/js/src/jsgc.cpp	Fri Oct 01 17:53:08 2010 -0700
>+++ b/js/src/jsgc.cpp	Fri Oct 01 20:59:55 2010 -0700
>@@ -2671,16 +2671,21 @@ js_GC(JSContext *cx, JSGCInvocationKind 
>      * should suppress that final collection or there may be shutdown leaks,
>      * or runtime bloat until the next context is created.
>      */
>     if (rt->state != JSRTS_UP && gckind != GC_LAST_CONTEXT)
>         return;
> 
>     RecordNativeStackTopForGC(cx);
> 
>+#ifdef DEBUG
>+    ConservativeGCThreadData *ctd = &JS_THREAD_DATA(cx)->conservativeGC;
>+    JS_ASSERT(JS_CHECK_STACK_SIZE(cx->stackLimit, ctd->nativeStackTop));
>+#endif

nativeStackTop can be the stack at the moment of SuspendRequest with js_GC called outside a request with much deeper stack. So the check above is not suitable for proper stack space detection. A solution could be to add to jscntxt.h an inline like (any better name is welcomed here):

inline void 
AssertNativeStackSpace(JSContext *cx) 
{
#ifdef DEBUG
    int stackDummy;
    JS_ASSERT(JS_CHECK_STACK_SIZE(cx->stackLimit, &stackDummy));
#endif
}

Comment 6

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 7

[Gregor Wagner [:gwagner]]

this passes now tryserver and I hit this assertion for the testcase in bug 600139.

Comment 8

[Gregor Wagner [:gwagner]]

http://hg.mozilla.org/tracemonkey/rev/fd4510c77054

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/fd4510c77054