Last Comment Bug 701761 - Assertion failure: !rt->gcRunning, at jsgc.cpp:2078
: Assertion failure: !rt->gcRunning, at jsgc.cpp:2078
Status: RESOLVED FIXED
js-triage-done
: assertion, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Linux
: -- critical (vote)
: mozilla11
Assigned To: [PTO to Dec5] Bill McCloskey (:billm)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: langfuzz
  Show dependency treegraph
 
Reported: 2011-11-11 09:49 PST by Christian Holler (:decoder)
Modified: 2012-02-01 13:57 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
shell testcase, unpack, chdir and run main.js with options "-n -m" (1.09 KB, application/x-compressed-tar)
2011-11-11 09:49 PST, Christian Holler (:decoder)
no flags Details
fix (802 bytes, patch)
2011-11-11 17:29 PST, [PTO to Dec5] Bill McCloskey (:billm)
cdleary: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2011-11-11 09:49:39 PST
Created attachment 573837 [details]
shell testcase, unpack, chdir and run main.js with options "-n -m"

The attached test asserts on mozilla-central revision 50c1bcb49c76 (options -m -n).

S-s for now due to GC relatedness.
Comment 1 Brian Hackett (:bhackett) 2011-11-11 09:51:41 PST
Can you paste the stack?
Comment 2 Christian Holler (:decoder) 2011-11-11 10:18:21 PST
GDB stack:

#1  0x00000000005c0dd6 in CrashInJS () at /srv/repos/mozilla-central/js/src/jsutil.cpp:95
#2  0x00000000005c0e2e in JS_Assert (s=0x78f327 "!rt->gcRunning", file=0x78ec30 "/srv/repos/mozilla-central/js/src/jsgc.cpp", ln=2078)
    at /srv/repos/mozilla-central/js/src/jsutil.cpp:103
#3  0x00000000004aca82 in js::TriggerGC (rt=0x7ffff7f72010, reason=js::gcstats::TOOMUCHMALLOC) at /srv/repos/mozilla-central/js/src/jsgc.cpp:2078
#4  0x0000000000470bad in JSRuntime::onTooMuchMalloc (this=0x7ffff7f72010) at /srv/repos/mozilla-central/js/src/jscntxt.cpp:1561
#5  0x000000000041aa65 in JSRuntime::updateMallocCounter (this=0x7ffff7f72010, nbytes=384) at ../../jscntxt.h:808
#6  0x000000000041a935 in JSRuntime::malloc_ (this=0x7ffff7f72010, bytes=384, cx=0x0) at ../../jscntxt.h:741
#7  0x000000000047190b in js::RuntimeAllocPolicy::malloc_ (this=0xb61a80, bytes=384) at /srv/repos/mozilla-central/js/src/jscntxt.h:2447
#8  0x0000000000472df2 in js::detail::HashTable<js::HashMapEntry<JSAtom*, js::detail::RegExpPrivate*>, js::HashMap<JSAtom*, js::detail::RegExpPrivate*, js::DefaultHasher<JSAtom*>, js::RuntimeAllocPolicy>::MapHashPolicy, js::RuntimeAllocPolicy>::createTable (alloc=..., capacity=16) at ./dist/include/js/HashTable.h:342
#9  0x00000000005a2ace in js::detail::HashTable<js::HashMapEntry<JSAtom*, js::detail::RegExpPrivate*>, js::HashMap<JSAtom*, js::detail::RegExpPrivate*, js::DefaultHasher<JSAtom*>, js::RuntimeAllocPolicy>::MapHashPolicy, js::RuntimeAllocPolicy>::changeTableSize (this=0xb61a80, deltaLog2=-1) at ./dist/include/js/HashTable.h:556
#10 0x00000000005a247a in js::detail::HashTable<js::HashMapEntry<JSAtom*, js::detail::RegExpPrivate*>, js::HashMap<JSAtom*, js::detail::RegExpPrivate*, js::DefaultHasher<JSAtom*>, js::RuntimeAllocPolicy>::MapHashPolicy, js::RuntimeAllocPolicy>::checkUnderloaded (this=0xb61a80) at ./dist/include/js/HashTable.h:598
#11 0x00000000005a17e1 in js::detail::HashTable<js::HashMapEntry<JSAtom*, js::detail::RegExpPrivate*>, js::HashMap<JSAtom*, js::detail::RegExpPrivate*, js::DefaultHasher<JSAtom*>, js::RuntimeAllocPolicy>::MapHashPolicy, js::RuntimeAllocPolicy>::remove (this=0xb61a80, p=...) at ./dist/include/js/HashTable.h:766
#12 0x00000000005a0065 in js::HashMap<JSAtom*, js::detail::RegExpPrivate*, js::DefaultHasher<JSAtom*>, js::RuntimeAllocPolicy>::remove (this=0xb61a80, p=...)
    at ./dist/include/js/HashTable.h:1012
#13 0x000000000059dc76 in js::detail::RegExpPrivate::decref (this=0xb53560, cx=0xb40ee0) at /srv/repos/mozilla-central/js/src/vm/RegExpObject-inl.h:486
#14 0x000000000066499a in js::RegExpObject::purge (this=0x7ffff6007230, cx=0xb40ee0) at ../vm/RegExpObject-inl.h:139
#15 0x00000000006632c5 in regexp_trace (trc=0xb59760, obj=0x7ffff6007230) at /srv/repos/mozilla-central/js/src/vm/RegExpObject.cpp:345
#16 0x00000000004c0a32 in js::gc::MarkChildren (trc=0xb59760, obj=0x7ffff6007230) at /srv/repos/mozilla-central/js/src/jsgcmark.cpp:869
#17 0x00000000004c1615 in js::TraceChildren (trc=0xb59760, thing=0x7ffff6007230, kind=JSTRACE_OBJECT) at /srv/repos/mozilla-central/js/src/jsgcmark.cpp:1083
#18 0x0000000000434675 in JS_TraceChildren (trc=0xb59760, thing=0x7ffff6007230, kind=JSTRACE_OBJECT) at /srv/repos/mozilla-central/js/src/jsapi.cpp:2334
#19 0x00000000004afb28 in js::gc::EndVerifyBarriers (cx=0xb40ee0) at /srv/repos/mozilla-central/js/src/jsgc.cpp:3552
#20 0x00000000004ae22f in AutoVerifyBarriers::AutoVerifyBarriers (this=0x7fffffffb310, cx=0xb40ee0) at /srv/repos/mozilla-central/js/src/jsgc.cpp:2985
#21 0x00000000004ae2f9 in js_GC (cx=0xb40ee0, comp=0x0, gckind=GC_NORMAL, reason=js::gcstats::TOOMUCHMALLOC) at /srv/repos/mozilla-central/js/src/jsgc.cpp:2988
#22 0x000000000047022b in js_InvokeOperationCallback (cx=0xb40ee0) at /srv/repos/mozilla-central/js/src/jscntxt.cpp:1211
#23 0x00000000004702bc in js_HandleExecutionInterrupt (cx=0xb40ee0) at /srv/repos/mozilla-central/js/src/jscntxt.cpp:1268
#24 0x00000000004ecfff in js::Interpret (cx=0xb40ee0, entryFrame=0x7ffff63fb140, interpMode=js::JSINTERP_NORMAL) at /srv/repos/mozilla-central/js/src/jsinterp.cpp:2221
#25 0x000000000066df4a in js::mjit::EnterMethodJIT (cx=0xb40ee0, fp=0x7ffff63fb140, code=0x7ffff7f22398, stackLimit=0x7ffff67db000, partial=false)
    at /srv/repos/mozilla-central/js/src/methodjit/MethodJIT.cpp:1094


Looking at the stack more closely, this could be related to issue 701387. But this test still reproduces on inbound where the fix for 701387 already landed. Ccing cdleary also to check this.
Comment 3 [PTO to Dec5] Bill McCloskey (:billm) 2011-11-11 12:49:15 PST
This is a bug with the write barriers. It's only triggered with gczeal(n) for n >= 4. Until incremental GC lands, bugs like this are not security critical. You can tell them apart because js::gc::EndVerifyBarriers is on the stack.
Comment 4 [PTO to Dec5] Bill McCloskey (:billm) 2011-11-11 17:29:36 PST
Created attachment 573973 [details] [diff] [review]
fix

I think we only want to purge during a GC.

Is it always safe to purge? For example, we could get here because of the cycle collector. Do we need to fix this for FF10?
Comment 5 [PTO to Dec5] Bill McCloskey (:billm) 2011-11-11 17:32:34 PST
It's also a little worrying that removing from a hashtable could cause memory allocation. It seems like a normal GC could still hit this assert. The same goes for weak maps, which remove elements during sweeping.
Comment 6 Chris Leary [:cdleary] (not checking bugmail) 2011-11-13 14:20:28 PST
Comment on attachment 573973 [details] [diff] [review]
fix

Review of attachment 573973 [details] [diff] [review]:
-----------------------------------------------------------------

Purging started in FF11. In FF10 the regexps are lazy but don't purge.

Bill, do you think we can we do a removeWithoutResize during tracing that will just avoid the underloaded table check?
Comment 7 [PTO to Dec5] Bill McCloskey (:billm) 2011-11-14 11:07:47 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee792c270e4f

> Bill, do you think we can we do a removeWithoutResize during tracing that will just avoid the > underloaded table check?

I'm not sure what the right thing to do is here. It would probably be a bad idea never to resize. At least for weakmaps, that might prevent a weakmap that goes from big to small from being resized at all. This morning I was thinking of ways of shrinking a hashtable without additional allocations. If the load factor is <= 1/2, you can do it in linear time. However, I'm not sure this is worth the effort. Maybe we should just tolerate malloc during GCs? I'm guessing there are a fair number of other places where it happens.

Luke, what do you think?
Comment 8 Luke Wagner [:luke] 2011-11-14 14:25:16 PST
From IRL: the in-place shrink when underloaded algo seems like it could be a bit slower, but not so much that we couldn't do it.  In general, the malloc is "safe" since its from the RuntimeAllocPolicy and the error, if it occurs, does not get reported (remove is infallible and transactionally handles oom).
Comment 9 Ed Morley [:emorley] 2011-11-14 19:35:31 PST
https://hg.mozilla.org/mozilla-central/rev/ee792c270e4f
Comment 10 Robert Kaiser 2011-11-15 09:10:21 PST
cdleary says this should fix some of the crashes I filed recently. Which ones of bug 701399, bug 701396, bug 702352 should this fix? What is our plan for Aurora, where (most of) those are happening as well?

Note You need to log in before you can comment on or make changes to this bug.