Bug 890968 - Part 3: RAII wrapper for inspecting contents of JSStrings in a thread safe way. (r=nmatsakis)

js/src/ion/IonFrames.cpp

@@ -586,8 +586,9 @@ HandleParallelFailure(ResumeFromException *rfe)
     ForkJoinSlice *slice = ForkJoinSlice::Current();
     IonFrameIterator iter(slice->perThreadData->ionTop);
 
+    parallel::Spew(parallel::SpewBailouts, "Bailing from VM reentry");
+
     while (!iter.isEntry()) {
-        parallel::Spew(parallel::SpewBailouts, "Bailing from VM reentry");
         if (iter.isScripted()) {
             slice->bailoutRecord->setCause(ParallelBailoutFailedIC,
                                            iter.script(), iter.script(), NULL);

js/src/ion/ParallelFunctions.cpp

@@ -256,17 +256,15 @@ do {
 } while(0)
 
 static ParallelResult
-ParCompareStrings(ForkJoinSlice *slice, JSString *str1,
-                  JSString *str2, int32_t *res)
+ParCompareStrings(ForkJoinSlice *slice, JSString *left, JSString *right, int32_t *res)
 {
-    if (!str1->isLinear())
-        return TP_RETRY_SEQUENTIALLY;
-    if (!str2->isLinear())
-        return TP_RETRY_SEQUENTIALLY;
-    JSLinearString &linearStr1 = str1->asLinear();
-    JSLinearString &linearStr2 = str2->asLinear();
-    if (!CompareChars(linearStr1.chars(), linearStr1.length(),
-                      linearStr2.chars(), linearStr2.length(),
+    ScopedThreadSafeStringInspector leftInspector(left);
+    ScopedThreadSafeStringInspector rightInspector(right);
+    if (!leftInspector.ensureChars(slice) || !rightInspector.ensureChars(slice))
+        return TP_FATAL;
+
+    if (!CompareChars(leftInspector.chars(), left->length(),
+                      rightInspector.chars(), right->length(),
                       res))
         return TP_FATAL;
 

js/src/vm/String.cpp

@@ -474,23 +474,13 @@ js::ConcatStringsPure(ThreadSafeContext *cx, JSString *left, JSString *right)
 
         jschar *buf = str->init(wholeLength);
 
-        if (const jschar *leftChars = left->maybeChars()) {
-            PodCopy(buf, leftChars, leftLen);
-        } else {
-            ScopedJSFreePtr<jschar> chars;
-            if (!left->getCharsNonDestructive(cx, chars))
-                return NULL;
-            PodCopy(buf, chars.get(), leftLen);
-        }
+        ScopedThreadSafeStringInspector leftInspector(left);
+        ScopedThreadSafeStringInspector rightInspector(right);
+        if (!leftInspector.ensureChars(cx) || !rightInspector.ensureChars(cx))
+            return NULL;
 
-        if (const jschar *rightChars = right->maybeChars()) {
-            PodCopy(buf + leftLen, rightChars, rightLen);
-        } else {
-            ScopedJSFreePtr<jschar> chars;
-            if (!right->getCharsNonDestructive(cx, chars))
-                return NULL;
-            PodCopy(buf + leftLen, chars.get(), rightLen);
-        }
+        PodCopy(buf, leftInspector.chars(), leftLen);
+        PodCopy(buf + leftLen, rightInspector.chars(), rightLen);
 
         buf[wholeLength] = 0;
         return str;
@@ -611,6 +601,30 @@ JSFlatString::isIndexSlow(uint32_t *indexp) const
     return false;
 }
 
+bool
+ScopedThreadSafeStringInspector::ensureChars(ThreadSafeContext *cx)
+{
+    if (chars_)
+        return true;
+
+    if (cx->isJSContext()) {
+        JSLinearString *linear = str_->ensureLinear(cx->asJSContext());
+        if (!linear)
+            return false;
+        chars_ = linear->chars();
+    } else {
+        chars_ = str_->maybeChars();
+        if (!chars_) {
+            if (!str_->getCharsNonDestructive(cx, scopedChars_))
+                return false;
+            chars_ = scopedChars_;
+        }
+    }
+
+    JS_ASSERT(chars_);
+    return true;
+}
+
 /*
  * Set up some tools to make it easier to generate large tables. After constant
  * folding, for each n, Rn(0) is the comma-separated list R(0), R(1), ..., R(2^n-1).

js/src/vm/String.h

@@ -802,6 +802,43 @@ JS_STATIC_ASSERT(sizeof(JSAtom) == sizeof(JSString));
 
 namespace js {
 
+/*
+ * Thread safe RAII wrapper for inspecting the contents of JSStrings. The
+ * thread safe operations such as |getCharsNonDestructive| require allocation
+ * of a char array. This allocation is not always required, such as when the
+ * string is already linear. This wrapper makes dealing with this detail more
+ * convenient by encapsulating the allocation logic.
+ *
+ * As the name suggests, this class is scoped. Return values from chars() and
+ * range() may not be valid after the inspector goes out of scope.
+ */
+
+class ScopedThreadSafeStringInspector
+{
+  private:
+    JSString *str_;
+    ScopedJSFreePtr<jschar> scopedChars_;
+    const jschar *chars_;
+
+  public:
+    ScopedThreadSafeStringInspector(JSString *str)
+      : str_(str),
+        chars_(NULL)
+    { }
+
+    bool ensureChars(ThreadSafeContext *cx);
+
+    const jschar *chars() {
+        JS_ASSERT(chars_);
+        return chars_;
+    }
+
+    JS::TwoByteChars range() {
+        JS_ASSERT(chars_);
+        return JS::TwoByteChars(chars_, str_->length());
+    }
+};
+
 class StaticStrings
 {
   private: