Last Comment Bug 534349 - Assertion Failure: New Context, Property Cache Enable, Concurrent GC
: Assertion Failure: New Context, Property Cache Enable, Concurrent GC
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: P2 major (vote)
: ---
Assigned To: general
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-12-11 17:06 PST by David Barbour
Modified: 2012-02-08 15:46 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments

Description David Barbour 2009-12-11 17:06:02 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5
Build Identifier: SpiderMonkey 1.8.0rc1

SpiderMonkey emits assertion failure on PropertyCache Enable operation performed from a Garbage Collection (js_GC) while another thread is creating a context (JS_NewContext).

Problem was isolated: js_GC has a pair of steps that iterate through all contexts, first to disable property caches, then later to enable property caches. However, the set of contexts so iterated is not atomic... that is, there may be a 'new' context during when enabling property caches that was not present when disabling them.

Reproducible: Sometimes

Steps to Reproduce:
1. Pump GC Zeal to 2
2. Start one thread creating many new GC things
3. Create a new context in another thread.
4. Repeat until race-condition is encountered.



Suggest fix: in jscntxt.c, js_SetContextThread, suggest performing a JS_LOCK_GC prior to performing any actions that manipulate propertyCache or affect which thread is assigned to a given cx. This also means locking around the memset for the propertyCache (esp. since a thread might be associated with more than one context, that risks another crash).
Comment 1 Andreas Gal :gal 2009-12-12 01:31:21 PST
David, could you attach your test case? (if you have one)
Comment 2 Igor Bukanov 2009-12-12 11:57:35 PST
On 1.9.1 and later this was fixed in bug 437325.
Comment 3 Igor Bukanov 2009-12-12 12:02:28 PST
(In reply to comment #0)
> Suggest fix: in jscntxt.c, js_SetContextThread, suggest performing a JS_LOCK_GC
> prior to performing any actions that manipulate propertyCache or affect which
> thread is assigned to a given cx.

This also requires to call js_WaitForGC from inside the lock to ensure serialization against the GC.
Comment 4 David Barbour 2009-12-14 10:58:43 PST
Igor: I must assume you refer to the:
  if(rt->gcThread != cx->thread)
   while(rt->gcLevel > 0)
     JS_AWAIT_GC_DONE(rt);

There is no 'js_WaitForGC' or any variation I can find in the 1.8 branch, but the above pattern is pervasive.

Andreas: I have no clean unit-test case that doesn't come with the whole pile of dependencies for my project. I apologize for the inconvenience.
Comment 5 Robert Sayre 2010-01-25 16:07:12 PST
(In reply to comment #2)
> On 1.9.1 and later this was fixed in bug 437325.

Is this fixed?
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-02-08 15:46:24 PST
If this is fixed on 1.9.1+, it's as fixed as it's ever going to be.

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