Bug 986864. r=sfink, a=sledru
authorJon Coppeard <jcoppeard@mozilla.com>
Mon, 31 Mar 2014 10:56:23 +0100
changeset 183634 e6b88dfe88cd
parent 183633 81285325c7db
child 183635 6c1da25749a0
push id3427
push userryanvm@gmail.com
push date2014-04-04 13:06 +0000
Treeherderresults
reviewerssfink, sledru
bugs986864
milestone29.0
Bug 986864. r=sfink, a=sledru
js/src/jit-test/tests/gc/bug-986864.js
js/src/jsfun.cpp
js/src/jsscript.cpp
js/src/jsscript.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug-986864.js
@@ -0,0 +1,8 @@
+// |jit-test| slow
+function x() {}
+for (var j = 0; j < 9999; ++j) {
+    (function() {
+        x += x.watch("endsWith", ArrayBuffer);
+        return 0 >> Function(x)
+    })()
+}
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -1203,18 +1203,18 @@ JSFunction::createScriptForLazilyInterpr
             if (!lazy->maybeScript())
                 lazy->initScript(clonedScript);
             return true;
         }
 
         JS_ASSERT(lazy->source()->hasSourceData());
 
         // Parse and compile the script from source.
-        SourceDataCache::AutoSuppressPurge asp(cx);
-        const jschar *chars = lazy->source()->chars(cx, asp);
+        SourceDataCache::AutoHoldEntry holder;
+        const jschar *chars = lazy->source()->chars(cx, holder);
         if (!chars)
             return false;
 
         const jschar *lazyStart = chars + lazy->begin();
         size_t lazyLength = lazy->end() - lazy->begin();
 
         if (!frontend::CompileLazyFunction(cx, lazy, lazyStart, lazyLength))
             return false;
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -1147,67 +1147,122 @@ JSScript::loadSource(JSContext *cx, Scri
 
 JSFlatString *
 JSScript::sourceData(JSContext *cx)
 {
     JS_ASSERT(scriptSource()->hasSourceData());
     return scriptSource()->substring(cx, sourceStart(), sourceEnd());
 }
 
-SourceDataCache::AutoSuppressPurge::AutoSuppressPurge(JSContext *cx)
- : cache_(cx->runtime()->sourceDataCache)
+SourceDataCache::AutoHoldEntry::AutoHoldEntry()
+  : cache_(nullptr), source_(nullptr), charsToFree_(nullptr)
 {
-    oldValue_ = cache_.numSuppressPurges_++;
 }
 
-SourceDataCache::AutoSuppressPurge::~AutoSuppressPurge()
+void
+SourceDataCache::AutoHoldEntry::holdEntry(SourceDataCache *cache, ScriptSource *source)
 {
-    cache_.numSuppressPurges_--;
-    JS_ASSERT(cache_.numSuppressPurges_ == oldValue_);
+    // Initialise the holder for a specific cache and script source. This will
+    // hold on to the cached source chars in the event that the cache is purged.
+    JS_ASSERT(!cache_ && !source_ && !charsToFree_);
+    cache_ = cache;
+    source_ = source;
 }
 
+void
+SourceDataCache::AutoHoldEntry::deferDelete(const jschar *chars)
+{
+    // Take ownership of source chars now the cache is being purged. Remove our
+    // reference to the ScriptSource which might soon be destroyed.
+    JS_ASSERT(cache_ && source_ && !charsToFree_);
+    cache_ = nullptr;
+    source_ = nullptr;
+    charsToFree_ = chars;
+}
+
+SourceDataCache::AutoHoldEntry::~AutoHoldEntry()
+{
+    // The holder is going out of scope. If it has taken ownership of cached
+    // chars then delete them, otherwise unregister ourself with the cache.
+    if (charsToFree_) {
+        JS_ASSERT(!cache_ && !source_);
+        js_free(const_cast<jschar *>(charsToFree_));
+    } else if (cache_) {
+        JS_ASSERT(source_);
+        cache_->releaseEntry(*this);
+    }
+}
+
+void
+SourceDataCache::holdEntry(AutoHoldEntry &holder, ScriptSource *ss)
+{
+    JS_ASSERT(!holder_);
+    holder.holdEntry(this, ss);
+    holder_ = &holder;
+}
+
+void
+SourceDataCache::releaseEntry(AutoHoldEntry &holder)
+{
+    JS_ASSERT(holder_ == &holder);
+    holder_ = nullptr;
+}
+
 const jschar *
-SourceDataCache::lookup(ScriptSource *ss, const AutoSuppressPurge &asp)
+SourceDataCache::lookup(ScriptSource *ss, AutoHoldEntry &holder)
 {
-    JS_ASSERT(this == &asp.cache());
+    JS_ASSERT(!holder_);
     if (!map_)
         return nullptr;
-    if (Map::Ptr p = map_->lookup(ss))
+    if (Map::Ptr p = map_->lookup(ss)) {
+        holdEntry(holder, ss);
         return p->value();
+    }
     return nullptr;
 }
 
 bool
-SourceDataCache::put(ScriptSource *ss, const jschar *str, const AutoSuppressPurge &asp)
+SourceDataCache::put(ScriptSource *ss, const jschar *str, AutoHoldEntry &holder)
 {
-    JS_ASSERT(this == &asp.cache());
+    JS_ASSERT(!holder_);
 
     if (!map_) {
         map_ = js_new<Map>();
         if (!map_)
             return false;
 
         if (!map_->init()) {
             js_delete(map_);
             map_ = nullptr;
             return false;
         }
     }
 
-    return map_->put(ss, str);
+    if (!map_->put(ss, str))
+        return false;
+
+    holdEntry(holder, ss);
+    return true;
 }
 
 void
 SourceDataCache::purge()
 {
-    if (!map_ || numSuppressPurges_ > 0)
+    if (!map_)
         return;
 
-    for (Map::Range r = map_->all(); !r.empty(); r.popFront())
-        js_delete(const_cast<jschar*>(r.front().value()));
+    for (Map::Range r = map_->all(); !r.empty(); r.popFront()) {
+        const jschar *chars = r.front().value();
+        if (holder_ && r.front().key() == holder_->source()) {
+            holder_->deferDelete(chars);
+            holder_ = nullptr;
+        } else {
+            js_free(const_cast<jschar*>(chars));
+        }
+    }
 
     js_delete(map_);
     map_ = nullptr;
 }
 
 size_t
 SourceDataCache::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf)
 {
@@ -1218,59 +1273,59 @@ SourceDataCache::sizeOfExcludingThis(moz
             const jschar *v = r.front().value();
             n += mallocSizeOf(v);
         }
     }
     return n;
 }
 
 const jschar *
-ScriptSource::chars(JSContext *cx, const SourceDataCache::AutoSuppressPurge &asp)
+ScriptSource::chars(JSContext *cx, SourceDataCache::AutoHoldEntry &holder)
 {
     if (const jschar *chars = getOffThreadCompressionChars(cx))
         return chars;
     JS_ASSERT(ready());
 
 #ifdef USE_ZLIB
     if (compressed()) {
-        if (const jschar *decompressed = cx->runtime()->sourceDataCache.lookup(this, asp))
+        if (const jschar *decompressed = cx->runtime()->sourceDataCache.lookup(this, holder))
             return decompressed;
-      
+
         const size_t nbytes = sizeof(jschar) * (length_ + 1);
         jschar *decompressed = static_cast<jschar *>(js_malloc(nbytes));
         if (!decompressed)
             return nullptr;
 
         if (!DecompressString(data.compressed, compressedLength_,
                               reinterpret_cast<unsigned char *>(decompressed), nbytes)) {
             JS_ReportOutOfMemory(cx);
             js_free(decompressed);
             return nullptr;
         }
 
         decompressed[length_] = 0;
 
-        if (!cx->runtime()->sourceDataCache.put(this, decompressed, asp)) {
+        if (!cx->runtime()->sourceDataCache.put(this, decompressed, holder)) {
             JS_ReportOutOfMemory(cx);
             js_free(decompressed);
             return nullptr;
         }
 
         return decompressed;
     }
 #endif
     return data.source;
 }
 
 JSStableString *
 ScriptSource::substring(JSContext *cx, uint32_t start, uint32_t stop)
 {
     JS_ASSERT(start <= stop);
-    SourceDataCache::AutoSuppressPurge asp(cx);
-    const jschar *chars = this->chars(cx, asp);
+    SourceDataCache::AutoHoldEntry holder;
+    const jschar *chars = this->chars(cx, holder);
     if (!chars)
         return nullptr;
     JSFlatString *flatStr = js_NewStringCopyN<CanGC>(cx, chars + start, stop - start);
     if (!flatStr)
         return nullptr;
     return flatStr->ensureStable(cx);
 }
 
@@ -3357,22 +3412,22 @@ LazyScriptHashPolicy::match(JSScript *sc
         script->column() != lazy->column() ||
         script->getVersion() != lazy->version() ||
         script->sourceStart() != lazy->begin() ||
         script->sourceEnd() != lazy->end())
     {
         return false;
     }
 
-    SourceDataCache::AutoSuppressPurge asp(cx);
-
-    const jschar *scriptChars = script->scriptSource()->chars(cx, asp);
+    SourceDataCache::AutoHoldEntry holder;
+
+    const jschar *scriptChars = script->scriptSource()->chars(cx, holder);
     if (!scriptChars)
         return false;
 
-    const jschar *lazyChars = lazy->source()->chars(cx, asp);
+    const jschar *lazyChars = lazy->source()->chars(cx, holder);
     if (!lazyChars)
         return false;
 
     size_t begin = script->sourceStart();
     size_t length = script->sourceEnd() - begin;
     return !memcmp(scriptChars + begin, lazyChars + begin, length);
 }
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -326,38 +326,51 @@ typedef HashMap<JSScript *,
 class ScriptSource;
 
 class SourceDataCache
 {
     typedef HashMap<ScriptSource *,
                     const jschar *,
                     DefaultHasher<ScriptSource *>,
                     SystemAllocPolicy> Map;
-    Map *map_;
-    size_t numSuppressPurges_;
 
   public:
-    SourceDataCache() : map_(nullptr), numSuppressPurges_(0) {}
-
-    class AutoSuppressPurge
+    // Hold an entry in the source data cache and prevent it from being purged on GC.
+    class AutoHoldEntry
     {
-        SourceDataCache &cache_;
-        mozilla::DebugOnly<size_t> oldValue_;
+        SourceDataCache *cache_;
+        ScriptSource *source_;
+        const jschar *charsToFree_;
       public:
-        explicit AutoSuppressPurge(JSContext *cx);
-        ~AutoSuppressPurge();
-        SourceDataCache &cache() const { return cache_; }
+        explicit AutoHoldEntry();
+        ~AutoHoldEntry();
+      private:
+        void holdEntry(SourceDataCache *cache, ScriptSource *source);
+        void deferDelete(const jschar *chars);
+        ScriptSource *source() const { return source_; }
+        friend class SourceDataCache;
     };
 
-    const jschar *lookup(ScriptSource *ss, const AutoSuppressPurge &asp);
-    bool put(ScriptSource *ss, const jschar *chars, const AutoSuppressPurge &asp);
+  private:
+    Map *map_;
+    AutoHoldEntry *holder_;
+
+  public:
+    SourceDataCache() : map_(nullptr), holder_(nullptr) {}
+
+    const jschar *lookup(ScriptSource *ss, AutoHoldEntry &asp);
+    bool put(ScriptSource *ss, const jschar *chars, AutoHoldEntry &asp);
 
     void purge();
 
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf);
+
+  private:
+    void holdEntry(AutoHoldEntry &holder, ScriptSource *ss);
+    void releaseEntry(AutoHoldEntry &holder);
 };
 
 class ScriptSource
 {
     friend class SourceCompressionTask;
 
     union {
         // Before setSourceCopy or setSource are successfully called, this union
@@ -422,17 +435,17 @@ class ScriptSource
     uint32_t length() const {
         JS_ASSERT(hasSourceData());
         return length_;
     }
     bool argumentsNotIncluded() const {
         JS_ASSERT(hasSourceData());
         return argumentsNotIncluded_;
     }
-    const jschar *chars(JSContext *cx, const SourceDataCache::AutoSuppressPurge &asp);
+    const jschar *chars(JSContext *cx, SourceDataCache::AutoHoldEntry &asp);
     JSStableString *substring(JSContext *cx, uint32_t start, uint32_t stop);
     size_t sizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
     // XDR handling
     template <XDRMode mode>
     bool performXDR(XDRState<mode> *xdr);
 
     bool setFilename(ExclusiveContext *cx, const char *filename);