Bug 804857 - Allocate memory in the compression thread and have its clients check for error. r=njn
authorBenjamin Peterson <benjamin@python.org>
Wed, 24 Oct 2012 16:15:48 -0700
changeset 111433 dfb516a4afc21e4adcd8055bdbd6aca01cff9752
parent 111432 1a97b903917f16b93a258990f6940ef005924330
child 111434 1c27fd77dbaa2c71ff5d5985cabb883ee285601e
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersnjn
bugs804857
milestone19.0a1
Bug 804857 - Allocate memory in the compression thread and have its clients check for error. r=njn
js/src/frontend/BytecodeCompiler.cpp
js/src/jsscript.cpp
js/src/jsscript.h
--- a/js/src/frontend/BytecodeCompiler.cpp
+++ b/js/src/frontend/BytecodeCompiler.cpp
@@ -241,16 +241,19 @@ frontend::CompileScript(JSContext *cx, H
     if (Emit1(cx, &bce, JSOP_STOP) < 0)
         return NULL;
 
     if (!JSScript::fullyInitFromEmitter(cx, script, &bce))
         return NULL;
 
     bce.tellDebuggerAboutCompiledScript(cx);
 
+    if (!sct.complete())
+        return NULL;
+
     return script;
 }
 
 // Compile a JS function body, which might appear as the value of an event
 // handler attribute in an HTML <INPUT> tag, or in a Function() constructor.
 bool
 frontend::CompileFunctionBody(JSContext *cx, HandleFunction fun, CompileOptions options,
                               const AutoNameVector &formals, StableCharPtr chars, size_t length)
@@ -347,10 +350,13 @@ frontend::CompileFunctionBody(JSContext 
     }
 
     if (!SetSourceMap(cx, parser.tokenStream, ss, script))
         return false;
 
     if (!EmitFunctionScript(cx, &funbce, pn))
         return false;
 
+    if (!sct.complete())
+        return false;
+
     return true;
 }
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -919,67 +919,84 @@ SourceCompressorThread::finish()
     if (wakeup)
         PR_DestroyCondVar(wakeup);
     if (done)
         PR_DestroyCondVar(done);
     if (lock)
         PR_DestroyLock(lock);
 }
 
+bool
+SourceCompressorThread::internalCompress()
+{
+    JS_ASSERT(state == COMPRESSING);
+    JS_ASSERT(tok);
+
+    ScriptSource *ss = tok->ss;
+    JS_ASSERT(!ss->ready());
+    const size_t COMPRESS_THRESHOLD = 512;
+    size_t compressedLength = 0;
+    size_t nbytes = sizeof(jschar) * ss->length_;
+
+    // Memory allocation functions on JSRuntime and JSContext are not
+    // threadsafe. We have to use the js_* variants.
+    ss->data.compressed = static_cast<unsigned char *>(js_malloc(nbytes));
+    if (!ss->data.compressed)
+        return false;
+
+#ifdef USE_ZLIB
+    if (nbytes >= COMPRESS_THRESHOLD) {
+        Compressor comp(reinterpret_cast<const unsigned char *>(tok->chars),
+                        nbytes, ss->data.compressed);
+        if (comp.init()) {
+            while (!stop && comp.compressMore())
+                ;
+            compressedLength = comp.finish();
+            if (stop || compressedLength == nbytes)
+                compressedLength = 0;
+        }
+    }
+#endif
+    ss->compressedLength_ = compressedLength;
+    if (compressedLength == 0) {
+        PodCopy(ss->data.source, tok->chars, ss->length());
+    } else {
+        // Shrink the buffer to the size of the compressed data.
+        void *newmem = js_realloc(ss->data.compressed, compressedLength);
+        if (!newmem) {
+            js_free(ss->data.compressed);
+            ss->data.compressed = NULL;
+            return false;
+        }
+        ss->data.compressed = static_cast<unsigned char *>(newmem);
+    }
+    return true;
+}
+
 void
 SourceCompressorThread::threadLoop()
 {
     PR_Lock(lock);
     while (true) {
         switch (state) {
           case SHUTDOWN:
             PR_Unlock(lock);
             return;
           case IDLE:
             PR_WaitCondVar(wakeup, PR_INTERVAL_NO_TIMEOUT);
             break;
-          case COMPRESSING: {
-            JS_ASSERT(tok);
-            ScriptSource *ss = tok->ss;
-            JS_ASSERT(!ss->ready());
-            const size_t COMPRESS_THRESHOLD = 512;
-            size_t compressedLength = 0;
-#ifdef USE_ZLIB
-            size_t nbytes = sizeof(jschar) * ss->length();
-            if (nbytes >= COMPRESS_THRESHOLD) {
-                Compressor comp(reinterpret_cast<const unsigned char *>(tok->chars),
-                                nbytes, ss->data.compressed);
-                if (comp.init()) {
-                    while (!stop && comp.compressMore())
-                        ;
-                    compressedLength = comp.finish();
-                    if (stop || compressedLength == nbytes)
-                        compressedLength = 0;
-                }
-            }
-#endif
-            ss->compressedLength_ = compressedLength;
-            if (compressedLength == 0) {
-                PodCopy(ss->data.source, tok->chars, ss->length());
-            } else {
-                // Shrink the buffer to the size of the compressed data. The
-                // memory allocation functions on JSContext and JSRuntime are
-                // not threadsafe, so use js_realloc directly. We'll fix up the
-                // memory accounting of the runtime in waitOnCompression().
-                void *newmem = js_realloc(ss->data.compressed, compressedLength);
-                JS_ASSERT(newmem); // Reducing memory size shouldn't fail.
-                ss->data.compressed = static_cast<unsigned char *>(newmem);
-            }
+          case COMPRESSING:
+            if (!internalCompress())
+                tok->oom = true;
 
             // We hold the lock, so no one should have changed this.
             JS_ASSERT(state == COMPRESSING);
             state = IDLE;
             PR_NotifyCondVar(done);
             break;
-          }
         }
     }
 }
 
 void
 SourceCompressorThread::compress(SourceCompressionToken *sct)
 {
     if (tok)
@@ -1009,22 +1026,19 @@ SourceCompressorThread::waitOnCompressio
     tok = NULL;
     PR_Unlock(lock);
 
     JS_ASSERT(!saveTok->ss->ready());
 #ifdef DEBUG
     saveTok->ss->ready_ = true;
 #endif
 
-    // Update memory accounting if needed.
-    if (saveTok->ss->compressed()) {
-        ptrdiff_t delta = saveTok->ss->compressedLength_ - sizeof(jschar) * saveTok->ss->length();
-        JS_ASSERT(delta < 0);
-        saveTok->cx->runtime->updateMallocCounter(NULL, delta);
-    }
+    // Update memory accounting.
+    if (!saveTok->oom)
+        saveTok->cx->runtime->updateMallocCounter(NULL, saveTok->ss->computedSizeOfData());
 
     saveTok->ss = NULL;
     saveTok->chars = NULL;
 }
 
 void
 SourceCompressorThread::abort(SourceCompressionToken *userTok)
 {
@@ -1139,55 +1153,63 @@ ScriptSource::substring(JSContext *cx, u
     return js_NewStringCopyN(cx, chars + start, stop - start);
 }
 
 bool
 ScriptSource::setSourceCopy(JSContext *cx, StableCharPtr src, uint32_t length,
                             bool argumentsNotIncluded, SourceCompressionToken *tok)
 {
     JS_ASSERT(!hasSourceData());
-    const size_t nbytes = length * sizeof(jschar);
-    data.compressed = static_cast<unsigned char *>(cx->malloc_(nbytes));
-    if (!data.compressed)
-        return false;
     length_ = length;
     argumentsNotIncluded_ = argumentsNotIncluded;
 
 #ifdef JS_THREADSAFE
     if (tok && cx->runtime->useHelperThreads()) {
 #ifdef DEBUG
         ready_ = false;
 #endif
         tok->ss = this;
         tok->chars = src.get();
         cx->runtime->sourceCompressorThread.compress(tok);
     } else
 #endif
     {
+        data.source = cx->pod_malloc<jschar>(length);
+        if (!data.source)
+            return false;
         PodCopy(data.source, src.get(), length_);
     }
 
     return true;
 }
 
 void
 ScriptSource::setSource(const jschar *src, uint32_t length)
 {
     JS_ASSERT(!hasSourceData());
     length_ = length;
     JS_ASSERT(!argumentsNotIncluded_);
     data.source = const_cast<jschar *>(src);
 }
 
-void
-SourceCompressionToken::ensureReady()
+bool
+SourceCompressionToken::complete()
 {
+    JS_ASSERT_IF(!ss, !chars);
 #ifdef JS_THREADSAFE
-    cx->runtime->sourceCompressorThread.waitOnCompression(this);
+    if (ss) {
+        cx->runtime->sourceCompressorThread.waitOnCompression(this);
+        JS_ASSERT(!ss);
+    }
+    if (oom) {
+        JS_ReportOutOfMemory(cx);
+        return false;
+    }
 #endif
+    return true;
 }
 
 void
 SourceCompressionToken::abort()
 {
 #ifdef JS_THREADSAFE
     cx->runtime->sourceCompressorThread.abort(this);
 #endif
--- a/js/src/jsscript.h
+++ b/js/src/jsscript.h
@@ -1056,17 +1056,20 @@ struct ScriptSource
 
     // Source maps
     bool setSourceMap(JSContext *cx, jschar *sourceMapURL, const char *filename);
     const jschar *sourceMap();
     bool hasSourceMap() const { return sourceMap_ != NULL; }
 
   private:
     void destroy(JSRuntime *rt);
-    bool compressed() { return compressedLength_ != 0; }
+    bool compressed() const { return compressedLength_ != 0; }
+    size_t computedSizeOfData() const {
+        return compressed() ? compressedLength_ : sizeof(jschar) * length_;
+    }
 };
 
 class ScriptSourceHolder
 {
     JSRuntime *rt;
     ScriptSource *ss;
   public:
     ScriptSourceHolder(JSRuntime *rt, ScriptSource *ss)
@@ -1084,18 +1087,20 @@ class ScriptSourceHolder
 #ifdef JS_THREADSAFE
 /*
  * Background thread to compress JS source code. This happens only while parsing
  * and bytecode generation is happening in the main thread. If needed, the
  * compiler waits for compression to complete before returning.
  *
  * To use it, you have to have a SourceCompressionToken, tok, with tok.ss and
  * tok.chars set to the proper values. When the SourceCompressionToken is
- * destroyed, it makes sure the compression is complete. At this point tok.ss is
- * ready to be attached to the runtime.
+ * destroyed, it makes sure the compression is complete. If you are about to
+ * successfully exit the scope of tok, you should call and check the return
+ * value of SourceCompressionToken::complete(). It returns false if allocation
+ * errors occurred in the thread.
  */
 class SourceCompressorThread
 {
   private:
     enum {
         // The compression thread is in the process of compression some source.
         COMPRESSING,
         // The compression thread is not doing anything and available to
@@ -1112,16 +1117,17 @@ class SourceCompressorThread
     // uses it to notify the compression thread when it has source to be
     // compressed.
     PRCondVar *wakeup;
     // The main thread can block on this to wait for compression to finish.
     PRCondVar *done;
     // Flag which can be set by the main thread to ask compression to abort.
     volatile bool stop;
 
+    bool internalCompress();
     void threadLoop();
     static void compressorThread(void *arg);
 
   public:
     explicit SourceCompressorThread(JSRuntime *rt)
     : state(IDLE),
       tok(NULL),
       thread(NULL),
@@ -1139,27 +1145,26 @@ class SourceCompressorThread
 struct SourceCompressionToken
 {
     friend struct ScriptSource;
     friend class SourceCompressorThread;
   private:
     JSContext *cx;
     ScriptSource *ss;
     const jschar *chars;
+    bool oom;
   public:
     explicit SourceCompressionToken(JSContext *cx)
-      : cx(cx), ss(NULL), chars(NULL) {}
+       : cx(cx), ss(NULL), chars(NULL), oom(false) {}
     ~SourceCompressionToken()
     {
-        JS_ASSERT_IF(!ss, !chars);
-        if (ss)
-            ensureReady();
+        complete();
     }
 
-    void ensureReady();
+    bool complete();
     void abort();
 };
 
 extern void
 CallDestroyScriptHook(FreeOp *fop, js::RawScript script);
 
 extern const char *
 SaveScriptFilename(JSContext *cx, const char *filename);