Currently our OOM handling code does not unlock the atoms compartment when reporting memory allocation errors under AtomizeInline. That can lead to a double lock if the OOM callback could be triggered to call atoms-related API.
Another problem with atoms compartment locking is the usage of JSRuntime::atomsCompartmentIsLocked in RunLastDitchGC:
if (cx->compartment == rt->atomsCompartment && rt->atomsCompartmentIsLocked)
Here atomsCompartmentIsLocked is a global variable that, as we still support JS_THREADSAFE, can be mutated on other threads. Those mutations may not yet propagate to the current thread and RunLastDitchGC may see atomsCompartmentIsLocked set when the atoms compartment is in fact unlocked.
Created attachment 579200 [details] [diff] [review]
To fix the thread-safety-issue and to simplify the unlocking of atoms compartment the patch moves atomsLocked flag into JSContext. This emphasizes that the flag is just a way to pass from AtomizeInline an extra information to the RunLastDitchGC and js_ReportOOM functions that they should unlock the compartment.
As a bonus this allowed to simplify the AutoUnlockAtomsCompartment class usage and fix can-be-not-initialized warning with GCC 4.6 that comes from the class usage in jsgc.cpp without the patch. That is how I found the bug.