Leaving spurious out-of-memory errors. It's due to rt->gcPoke being cleared too soon if set, in the code near the top of js_GC. Patch coming up. /be

This bug's patch should go into the 1.0 branch as soon as the 1.5 final landing happens, if not sooner. /be

Poke before last ditch call to js_GC from js_AllocGCThing, and clear rt->gcPoke only after finishing garbage collection, so racing js_GCs see gcPoke set, do not bail with the early return if (!rt->gcPoke), and flow into the code that awaits the completion of the garbage collection that's already in progress. /be

Comment on attachment 95337 [details] [diff] [review] proposed fix jband sez sr=jband (thanks again to him for helping test the testcase from a SpiderMonkey embedder). /be

that's nice of him, but for one minor problem: 1.39 brendan%mozilla.org Jul 11 15:43 Out with the old, in with the new. http://www.mozilla.org/webtools/bonsai/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=reviewers.html&root=/cvsroot&subdir=mozilla-org/html/hacking&command=DIFF_FRAMESET&rev1=1.38&rev2=1.39 you should probably ask shaver to sr, or you could get some other sr to sr based on jband's word that it's good ;-) -- personally i'd probably ask jband to r= and get ye old random sr. otoh, it's possible i missed a nonpublished memo, that happens to me constantly. anyway, i checked, the document still contains this fragment (it had me worried for a moment): Super-review does not require domain expertise (module owners and peers supply that, usually), so the areas below are not pigeon-holes -- you can solicit a super-review from any reviewer on the list, but using area as a guide will get quicker results in the typical case.

Timeless, settle down. /be

Forgot all the other early return paths, which are now fused with a done: block at the bottom of js_GC that (if !(gcflags & GC_ALREADY_LOCKED)) unlocks rt->gcLock, then calls the JSGC_END callback (jband, this restores symmetry with JSGC_BEGIN, which was broken before -- can you review?). /be

The JSGC_END GC-callback case, by design, is not called for every racing GC attempt that synchronizes with the thread actually collecting garbage -- it must be called only on the thread actually collecting garbage, after it has finished and released rt->gcLock. My last patch gratuitously and overzealously (I was motivated by code-sharing interests, to fuse the 'if (!(gcflags & GC_ALREADY_LOCKED)) JS_UNLOCK_GC(rt);' statement) made all racing GC-threads call the GC-callback with JSGC_END. This would at best do unnecessary work and at worst break someone. JSGC_BEGIN is different because we want any thread trying to start GC, before it has found another thread already collecting garbage, to be able to cause an early return from js_GC, via a false return from the GC-callback. Looking for r= and (jband again) sr=. /be

Comment on attachment 96220 [details] [diff] [review] oops, don't break JSGC_END's semantic r=shaver

jband's around, I have hopes he'll stamp sr= after reading this diff. /be

Comment on attachment 96220 [details] [diff] [review] oops, don't break JSGC_END's semantic sr=jband looks reasonable to me

Fixed in the trunk. /be

Marking Verified -