Backed out changeset 685accd622f0 (bug 606065).

js/src/jsapi.cpp

@@ -633,6 +633,10 @@ JSRuntime::init(uint32 maxbytes)
         return false;
 #endif
 
+    deflatedStringCache = new js::DeflatedStringCache();
+    if (!deflatedStringCache || !deflatedStringCache->init())
+        return false;
+
     wrapObjectCallback = js::TransparentObjectWrapper;
 
 #ifdef JS_THREADSAFE
@@ -675,6 +679,11 @@ JSRuntime::~JSRuntime()
     js_FreeRuntimeScriptState(this);
     js_FinishAtomState(this);
 
+    /*
+     * Finish the deflated string cache after the last GC and after
+     * calling js_FinishAtomState, which finalizes strings.
+     */
+    delete deflatedStringCache;
 #if ENABLE_YARR_JIT
     delete regExpAllocator;
 #endif
@@ -1170,7 +1179,7 @@ bool
 JSAutoEnterCompartment::enter(JSContext *cx, JSObject *target)
 {
     JS_ASSERT(!call);
-    if (cx->compartment == target->compartment()) {
+    if (cx->compartment == target->getCompartment()) {
         call = reinterpret_cast<JSCrossCompartmentCall*>(1);
         return true;
     }
@@ -1224,8 +1233,8 @@ JS_TransplantWrapper(JSContext *cx, JSObject *wrapper, JSObject *target)
      * need to "move" the window from wrapper's compartment to target's
      * compartment.
      */
-    JSCompartment *destination = target->compartment();
-    if (wrapper->compartment() == destination) {
+    JSCompartment *destination = target->getCompartment();
+    if (wrapper->getCompartment() == destination) {
         // If the wrapper is in the same compartment as the destination, then
         // we know that we won't find wrapper in the destination's cross
         // compartment map and that the same object will continue to work.
@@ -1302,7 +1311,7 @@ JS_TransplantWrapper(JSContext *cx, JSObject *wrapper, JSObject *target)
             return NULL;
         if (!wrapper->swap(cx, tobj))
             return NULL;
-        wrapper->compartment()->crossCompartmentWrappers.put(targetv, wrapperv);
+        wrapper->getCompartment()->crossCompartmentWrappers.put(targetv, wrapperv);
     }
 
     return obj;
@@ -5070,7 +5079,7 @@ JS_NewString(JSContext *cx, char *bytes, size_t nbytes)
     }
 
     /* Hand off bytes to the deflated string cache, if possible. */
-    if (!cx->compartment->deflatedStringCache.setBytes(cx, str, bytes))
+    if (!cx->runtime->deflatedStringCache->setBytes(cx, str, bytes))
         cx->free(bytes);
     return str;
 }

js/src/jscntxt.cpp

@@ -2069,7 +2069,7 @@ JSContext::resetCompartment()
             return;
         }
     }
-    compartment = scopeobj->compartment();
+    compartment = scopeobj->getCompartment();
 }
 
 void

js/src/jscntxt.h

@@ -1406,6 +1406,8 @@ struct JSRuntime {
     js::Value           negativeInfinityValue;
     js::Value           positiveInfinityValue;
 
+    js::DeflatedStringCache *deflatedStringCache;
+
     JSString            *emptyString;
 
     /* List of active contexts sharing this runtime; protected by gcLock. */

js/src/jscntxtinlines.h

@@ -526,7 +526,7 @@ class CompartmentChecker
 
     void check(JSObject *obj) {
         if (obj)
-            check(obj->compartment());
+            check(obj->getCompartment());
     }
 
     void check(const js::Value &v) {

js/src/jscompartment.cpp

@@ -74,8 +74,7 @@ JSCompartment::init()
 #ifdef JS_GCMETER
     memset(&compartmentStats, 0, sizeof(JSGCArenaStats) * FINALIZE_LIMIT);
 #endif
-    return crossCompartmentWrappers.init() &&
-           deflatedStringCache.init();
+    return crossCompartmentWrappers.init();
 }
 
 bool
@@ -153,7 +152,7 @@ JSCompartment::wrap(JSContext *cx, Value *vp)
         if (!obj->getClass()->ext.innerObject) {
             obj = vp->toObject().unwrap(&flags);
             vp->setObject(*obj);
-            if (obj->compartment() == this)
+            if (obj->getCompartment() == this)
                 return true;
 
             if (cx->runtime->preWrapObjectCallback)
@@ -162,7 +161,7 @@ JSCompartment::wrap(JSContext *cx, Value *vp)
                 return false;
 
             vp->setObject(*obj);
-            if (obj->compartment() == this)
+            if (obj->getCompartment() == this)
                 return true;
         } else {
             if (cx->runtime->preWrapObjectCallback)

js/src/jscompartment.h

@@ -84,11 +84,6 @@ struct JS_FRIEND_API(JSCompartment) {
     JSObject            *anynameObject;
     JSObject            *functionNamespaceObject;
 
-    /*
-     * Deflated string cache for this compartment.
-     */
-    js::DeflatedStringCache deflatedStringCache;
-
     JSCompartment(JSRuntime *cx);
     ~JSCompartment();
 
@@ -139,7 +134,7 @@ class SwitchToCompartment : public PreserveCompartment {
     }
 
     SwitchToCompartment(JSContext *cx, JSObject *target) : PreserveCompartment(cx) {
-        cx->compartment = target->compartment();
+        cx->compartment = target->getCompartment();
     }
 };
 

js/src/jsfun.cpp

@@ -1591,7 +1591,7 @@ fun_getProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp)
             JSObject &caller = vp->toObject();
 
             /* Censor the caller if it is from another compartment. */
-            if (caller.compartment() != cx->compartment) {
+            if (caller.getCompartment() != cx->compartment) {
                 vp->setNull();
             } else if (caller.isFunction() && caller.getFunctionPrivate()->inStrictMode()) {
                 JS_ReportErrorFlagsAndNumber(cx, JSREPORT_ERROR, js_GetErrorMessage, NULL,

js/src/jsgc.cpp

@@ -2245,13 +2245,17 @@ MarkAndSweep(JSContext *cx, JSGCInvocationKind gckind GCTIMER_PARAM)
     }
     TIMESTAMP(sweepObjectEnd);
 
+    /*
+     * We sweep the deflated cache before we finalize the strings so the
+     * cache can safely use js_IsAboutToBeFinalized..
+     */
+    rt->deflatedStringCache->sweep(cx);
+
     for (JSCompartment **comp = rt->compartments.begin(); comp != rt->compartments.end(); comp++) {
-        JSCompartment *compartment = *comp;
-        compartment->deflatedStringCache.sweep(cx);
-        FinalizeArenaList<JSShortString>(compartment, cx, FINALIZE_SHORT_STRING);
-        FinalizeArenaList<JSString>(compartment, cx, FINALIZE_STRING);
+        FinalizeArenaList<JSShortString>(*comp, cx, FINALIZE_SHORT_STRING);
+        FinalizeArenaList<JSString>(*comp, cx, FINALIZE_STRING);
         for (unsigned i = FINALIZE_EXTERNAL_STRING0; i <= FINALIZE_EXTERNAL_STRING_LAST; ++i)
-            FinalizeArenaList<JSString>(compartment, cx, i);
+            FinalizeArenaList<JSString>(*comp, cx, i);
     }
 
     TIMESTAMP(sweepStringEnd);

js/src/jsobj.cpp

@@ -6150,7 +6150,7 @@ js_TraceObject(JSTracer *trc, JSObject *obj)
             (void) clasp->mark(cx, obj, trc);
     }
     if (clasp->flags & JSCLASS_IS_GLOBAL) {
-        JSCompartment *compartment = obj->compartment();
+        JSCompartment *compartment = obj->getCompartment();
         compartment->marked = true;
     }
 

js/src/jsstr.cpp

@@ -4163,15 +4163,47 @@ bufferTooSmall:
 
 namespace js {
 
+DeflatedStringCache::DeflatedStringCache()
+{
+#ifdef JS_THREADSAFE
+    lock = NULL;
+#endif
+}
+
 bool
 DeflatedStringCache::init()
 {
-    return map.init(32);
+#ifdef JS_THREADSAFE
+    JS_ASSERT(!lock);
+    lock = JS_NEW_LOCK();
+    if (!lock)
+        return false;
+#endif
+
+    /*
+     * Make room for 2K deflated strings that a typical browser session
+     * creates.
+     */
+    return map.init(2048);
+}
+
+DeflatedStringCache::~DeflatedStringCache()
+{
+#ifdef JS_THREADSAFE
+    if (lock)
+        JS_DESTROY_LOCK(lock);
+#endif
 }
 
 void
 DeflatedStringCache::sweep(JSContext *cx)
 {
+    /*
+     * We must take a lock even during the GC as JS_GetStringBytes() can be
+     * called outside the request.
+     */
+    JS_ACQUIRE_LOCK(lock);
+
     for (Map::Enum e(map); !e.empty(); e.popFront()) {
         JSString *str = e.front().key;
         if (IsAboutToBeFinalized(str)) {
@@ -4187,52 +4219,77 @@ DeflatedStringCache::sweep(JSContext *cx)
             js_free(bytes);
         }
     }
+
+    JS_RELEASE_LOCK(lock);
 }
 
 void
 DeflatedStringCache::remove(JSString *str)
 {
+    JS_ACQUIRE_LOCK(lock);
+
     Map::Ptr p = map.lookup(str);
     if (p) {
         js_free(p->value);
         map.remove(p);
     }
+
+    JS_RELEASE_LOCK(lock);
 }
 
 bool
 DeflatedStringCache::setBytes(JSContext *cx, JSString *str, char *bytes)
 {
+    JS_ACQUIRE_LOCK(lock);
+
     Map::AddPtr p = map.lookupForAdd(str);
     JS_ASSERT(!p);
-    if (!map.add(p, str, bytes)) {
+    bool ok = map.add(p, str, bytes);
+
+    JS_RELEASE_LOCK(lock);
+
+    if (!ok)
         js_ReportOutOfMemory(cx);
-        return false;
-    }
-    return true;
+    return ok;
 }
 
 char *
 DeflatedStringCache::getBytes(JSContext *cx, JSString *str)
 {
+    JS_ACQUIRE_LOCK(lock);
     Map::AddPtr p = map.lookupForAdd(str);
-    if (p && p->value)
-        return p->value;
+    char *bytes = p ? p->value : NULL;
+    JS_RELEASE_LOCK(lock);
 
-    char *bytes = js_DeflateString(cx, str->chars(), str->length());
+    if (bytes)
+        return bytes;
+
+    bytes = js_DeflateString(cx, str->chars(), str->length());
     if (!bytes)
         return NULL;
 
     /*
-     * 1. js_DeflateString does not mutate the map.
-     * 2. At most one thread is allowed to mutate a compartment at any
-     *    given time.
-     * 3. Each compartment has its own map.
-     *
-     * Hence, map was not changed since lookupForAdd() and we can use
-     * a regular add() here.
+     * In the single-threaded case we use the add method as js_DeflateString
+     * cannot mutate the map. In particular, it cannot run the GC that may
+     * delete entries from the map. But the JS_THREADSAFE version requires to
+     * deal with other threads adding the entries to the map.
      */
     char *bytesToFree = NULL;
-    if (!map.add(p, str, bytes)) {
+    JSBool ok;
+#ifdef JS_THREADSAFE
+    JS_ACQUIRE_LOCK(lock);
+    ok = map.relookupOrAdd(p, str, bytes);
+    if (ok && p->value != bytes) {
+        /* Some other thread has asked for str bytes .*/
+        JS_ASSERT(!strcmp(p->value, bytes));
+        bytesToFree = bytes;
+        bytes = p->value;
+    }
+    JS_RELEASE_LOCK(lock);
+#else  /* !JS_THREADSAFE */
+    ok = map.add(p, str, bytes);
+#endif
+    if (!ok) {
         bytesToFree = bytes;
         bytes = NULL;
         if (cx)
@@ -4253,6 +4310,7 @@ DeflatedStringCache::getBytes(JSContext *cx, JSString *str)
 const char *
 js_GetStringBytes(JSContext *cx, JSString *str)
 {
+    JSRuntime *rt;
     char *bytes;
 
     if (JSString::isUnitString(str)) {
@@ -4283,7 +4341,14 @@ js_GetStringBytes(JSContext *cx, JSString *str)
         return JSString::deflatedIntStringTable + ((str - JSString::hundredStringTable) * 4);
     }
 
-    return str->asCell()->compartment()->deflatedStringCache.getBytes(cx, str);
+    if (cx) {
+        rt = cx->runtime;
+    } else {
+        /* JS_GetStringBytes calls us with null cx. */
+        rt = GetGCThingRuntime(str);
+    }
+
+    return rt->deflatedStringCache->getBytes(cx, str);
 }
 
 /*

js/src/jsstr.h

@@ -1227,7 +1227,9 @@ namespace js {
 
 class DeflatedStringCache {
   public:
+    DeflatedStringCache();
     bool init();
+    ~DeflatedStringCache();
 
     void sweep(JSContext *cx);
     void remove(JSString *str);
@@ -1267,6 +1269,9 @@ class DeflatedStringCache {
     ::js_GetStringBytes(JSContext *cx, JSString *str);
 
     Map                 map;
+#ifdef JS_THREADSAFE
+    JSLock              *lock;
+#endif
 };
 
 } /* namespace js */

js/src/jswrapper.cpp

@@ -317,7 +317,7 @@ AutoCompartment::AutoCompartment(JSContext *cx, JSObject *target)
     : context(cx),
       origin(cx->compartment),
       target(target),
-      destination(target->compartment()),
+      destination(target->getCompartment()),
       input(cx),
       entered(false)
 {