Bug 979293 - Add a FrozenAtomSet to clarify how |permanentAtoms| works. r=bhackett.

This clarifies the two phases -- (a) initialization and (b) read-only use
-- that |permanentAtoms| goes through. It also gives some type-based protection
against potential misuse.

js/src/jsatom.cpp

@@ -97,6 +97,10 @@ const char js_with_str[]            = "with";
 // which create a small number of atoms.
 static const uint32_t JS_STRING_HASH_COUNT = 64;
 
+AtomSet::Ptr js::FrozenAtomSet::readonlyThreadsafeLookup(const AtomSet::Lookup &l) const {
+    return mSet->readonlyThreadsafeLookup(l);
+}
+
 struct CommonNameInfo
 {
     const char *str;
@@ -110,6 +114,9 @@ JSRuntime::initializeAtoms(JSContext *cx)
     if (!atoms_ || !atoms_->init(JS_STRING_HASH_COUNT))
         return false;
 
+    // |permanentAtoms| hasn't been created yet.
+    MOZ_ASSERT(!permanentAtoms);
+
     if (parentRuntime) {
         staticStrings = parentRuntime->staticStrings;
         commonNames = parentRuntime->commonNames;
@@ -119,10 +126,6 @@ JSRuntime::initializeAtoms(JSContext *cx)
         return true;
     }
 
-    permanentAtoms = cx->new_<AtomSet>();
-    if (!permanentAtoms || !permanentAtoms->init(JS_STRING_HASH_COUNT))
-        return false;
-
     staticStrings = cx->new_<StaticStrings>();
     if (!staticStrings || !staticStrings->init(cx))
         return false;
@@ -221,8 +224,8 @@ js::MarkPermanentAtoms(JSTracer *trc)
         rt->staticStrings->trace(trc);
 
     if (rt->permanentAtoms) {
-        for (AtomSet::Enum e(*rt->permanentAtoms); !e.empty(); e.popFront()) {
-            const AtomStateEntry &entry = e.front();
+        for (FrozenAtomSet::Range r(rt->permanentAtoms->all()); !r.empty(); r.popFront()) {
+            const AtomStateEntry &entry = r.front();
 
             JSAtom *atom = entry.asPtr();
             MarkPermanentAtom(trc, atom, "permanent_table");
@@ -264,21 +267,22 @@ JSRuntime::sweepAtoms()
 }
 
 bool
-JSRuntime::transformToPermanentAtoms()
+JSRuntime::transformToPermanentAtoms(JSContext *cx)
 {
     MOZ_ASSERT(!parentRuntime);
 
     // All static strings were created as permanent atoms, now move the contents
     // of the atoms table into permanentAtoms and mark each as permanent.
 
-    MOZ_ASSERT(permanentAtoms && permanentAtoms->empty());
+    MOZ_ASSERT(!permanentAtoms);
+    permanentAtoms = cx->new_<FrozenAtomSet>(atoms_);   // takes ownership of atoms_
 
-    AtomSet *temp = atoms_;
-    atoms_ = permanentAtoms;
-    permanentAtoms = temp;
+    atoms_ = cx->new_<AtomSet>();
+    if (!atoms_ || !atoms_->init(JS_STRING_HASH_COUNT))
+        return false;
 
-    for (AtomSet::Enum e(*permanentAtoms); !e.empty(); e.popFront()) {
-        AtomStateEntry entry = e.front();
+    for (FrozenAtomSet::Range r(permanentAtoms->all()); !r.empty(); r.popFront()) {
+        AtomStateEntry entry = r.front();
         JSAtom *atom = entry.asPtr();
         atom->morphIntoPermanentAtom();
     }
@@ -296,6 +300,7 @@ AtomIsInterned(JSContext *cx, JSAtom *atom)
     AtomHasher::Lookup lookup(atom);
 
     /* Likewise, permanent strings are considered to be interned. */
+    MOZ_ASSERT(cx->isPermanentAtomsInitialized());
     AtomSet::Ptr p = cx->permanentAtoms().readonlyThreadsafeLookup(lookup);
     if (p)
         return true;
@@ -320,9 +325,16 @@ AtomizeAndCopyChars(ExclusiveContext *cx, const CharT *tbchars, size_t length, I
 
     AtomHasher::Lookup lookup(tbchars, length);
 
-    AtomSet::Ptr pp = cx->permanentAtoms().readonlyThreadsafeLookup(lookup);
-    if (pp)
-        return pp->asPtr();
+    // Note: when this function is called while the permanent atoms table is
+    // being initialized (in initializeAtoms()), |permanentAtoms| is not yet
+    // initialized so this lookup is always skipped. Only once
+    // transformToPermanentAtoms() is called does |permanentAtoms| get
+    // initialized and then this lookup will go ahead.
+    if (cx->isPermanentAtomsInitialized()) {
+        AtomSet::Ptr pp = cx->permanentAtoms().readonlyThreadsafeLookup(lookup);
+        if (pp)
+            return pp->asPtr();
+    }
 
     AutoLockForExclusiveAccess lock(cx);
 
@@ -377,6 +389,7 @@ js::AtomizeString(ExclusiveContext *cx, JSString *str,
         AtomHasher::Lookup lookup(&atom);
 
         /* Likewise, permanent atoms are always interned. */
+        MOZ_ASSERT(cx->isPermanentAtomsInitialized());
         AtomSet::Ptr p = cx->permanentAtoms().readonlyThreadsafeLookup(lookup);
         if (p)
             return &atom;

js/src/jsatom.h

@@ -117,6 +117,30 @@ struct AtomHasher
 
 typedef HashSet<AtomStateEntry, AtomHasher, SystemAllocPolicy> AtomSet;
 
+// This class is a wrapper for AtomSet that is used to ensure the AtomSet is
+// not modified. It should only expose read-only methods from AtomSet.
+// Note however that the atoms within the table can be marked during GC.
+class FrozenAtomSet
+{
+    AtomSet *mSet;
+
+public:
+    // This constructor takes ownership of the passed-in AtomSet.
+    explicit FrozenAtomSet(AtomSet *set) { mSet = set; }
+
+    ~FrozenAtomSet() { js_delete(mSet); }
+
+    AtomSet::Ptr readonlyThreadsafeLookup(const AtomSet::Lookup &l) const;
+
+    size_t sizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
+        return mSet->sizeOfIncludingThis(mallocSizeOf);
+    }
+
+    typedef AtomSet::Range Range;
+
+    AtomSet::Range all() const { return mSet->all(); }
+};
+
 class PropertyName;
 
 }  /* namespace js */

js/src/jscntxt.cpp

@@ -126,7 +126,7 @@ js::NewContext(JSRuntime *rt, size_t stackChunkSize)
             ok = rt->initSelfHosting(cx);
 
         if (ok && !rt->parentRuntime)
-            ok = rt->transformToPermanentAtoms();
+            ok = rt->transformToPermanentAtoms(cx);
 
         JS_EndRequest(cx);
 

js/src/jscntxt.h

@@ -183,7 +183,8 @@ class ExclusiveContext : public ContextFriendFields,
     // Accessors for immutable runtime data.
     JSAtomState &names() { return *runtime_->commonNames; }
     StaticStrings &staticStrings() { return *runtime_->staticStrings; }
-    AtomSet &permanentAtoms() { return *runtime_->permanentAtoms; }
+    bool isPermanentAtomsInitialized() { return !!runtime_->permanentAtoms; }
+    FrozenAtomSet &permanentAtoms() { return *runtime_->permanentAtoms; }
     WellKnownSymbols &wellKnownSymbols() { return *runtime_->wellKnownSymbols; }
     const JS::AsmJSCacheOps &asmJSCacheOps() { return runtime_->asmJSCacheOps; }
     PropertyName *emptyString() { return runtime_->emptyString; }

js/src/vm/Runtime.h

@@ -1254,9 +1254,11 @@ struct JSRuntime : public JS::shadow::Runtime,
     JSAtomState *commonNames;
 
     // All permanent atoms in the runtime, other than those in staticStrings.
-    js::AtomSet *permanentAtoms;
+    // Unlike |atoms_|, access to this does not require
+    // AutoLockForExclusiveAccess because it is frozen and thus read-only.
+    js::FrozenAtomSet *permanentAtoms;
 
-    bool transformToPermanentAtoms();
+    bool transformToPermanentAtoms(JSContext *cx);
 
     // Cached well-known symbols (ES6 rev 24 6.1.5.1). Like permanent atoms,
     // these are shared with the parentRuntime, if any.