534349 - Assertion Failure: New Context, Property Cache Enable, Concurrent GC

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[David Barbour]

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]

David, could you attach your test case? (if you have one)

Comment 2

[Igor Bukanov]

On 1.9.1 and later this was fixed in bug 437325.

Comment 3

[Igor Bukanov]

(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]

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]

(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]]

If this is fixed on 1.9.1+, it's as fixed as it's ever going to be.