Bug 841646 - Part 2: Add locks around |strings| table access to avoid races. r=jandem

js/src/vm/SPSProfiler.cpp

@@ -28,6 +28,11 @@ SPSProfiler::SPSProfiler(JSRuntime *rt)
     enabled_(false)
 {
     JS_ASSERT(rt != nullptr);
+#ifdef JS_THREADSAFE
+    lock_ = PR_NewLock();
+    if (lock_ == nullptr)
+        MOZ_CRASH("Couldn't allocate lock!");
+#endif
 }
 
 SPSProfiler::~SPSProfiler()
@@ -36,11 +41,15 @@ SPSProfiler::~SPSProfiler()
         for (ProfileStringMap::Enum e(strings); !e.empty(); e.popFront())
             js_free(const_cast<char *>(e.front().value()));
     }
+#ifdef JS_THREADSAFE
+    PR_DestroyLock(lock_);
+#endif
 }
 
 void
 SPSProfiler::setProfilingStack(ProfileEntry *stack, uint32_t *size, uint32_t max)
 {
+    AutoSPSLock lock(lock_);
     JS_ASSERT_IF(size_ && *size_ != 0, !enabled());
     if (!strings.initialized())
         strings.init();
@@ -79,6 +88,7 @@ SPSProfiler::enable(bool enabled)
 const char*
 SPSProfiler::profileString(JSScript *script, JSFunction *maybeFun)
 {
+    AutoSPSLock lock(lock_);
     JS_ASSERT(strings.initialized());
     ProfileStringMap::AddPtr s = strings.lookupForAdd(script);
     if (s)
@@ -103,6 +113,7 @@ SPSProfiler::onScriptFinalized(JSScript *script)
      * off, we still want to remove the string, so no check of enabled() is
      * done.
      */
+    AutoSPSLock lock(lock_);
     if (!strings.initialized())
         return;
     if (ProfileStringMap::Ptr entry = strings.lookup(script)) {

js/src/vm/SPSProfiler.h

@@ -12,6 +12,7 @@
 
 #include <stddef.h>
 
+#include "jslock.h"
 #include "jsscript.h"
 
 #include "js/ProfilingStack.h"
@@ -123,6 +124,7 @@ class SPSProfiler
     uint32_t             max_;
     bool                 slowAssertions;
     uint32_t             enabled_;
+    PRLock               *lock_;
 
     const char *allocProfileString(JSScript *script, JSFunction *function);
     void push(const char *string, void *sp, JSScript *script, jsbytecode *pc);
@@ -185,14 +187,51 @@ class SPSProfiler
     void onScriptFinalized(JSScript *script);
 
     /* meant to be used for testing, not recommended to call in normal code */
-    size_t stringsCount() { return strings.count(); }
+    size_t stringsCount();
-    void stringsReset() { strings.clear(); }
+    void stringsReset();
 
     uint32_t *addressOfEnabled() {
         return &enabled_;
     }
 };
 
+/*
+ * This class is used to make sure the strings table
+ * is only accessed on one thread at a time.
+ */
+class AutoSPSLock
+{
+  public:
+#ifdef JS_THREADSAFE
+    AutoSPSLock(PRLock *lock)
+    {
+        MOZ_ASSERT(lock, "Parameter should not be null!");
+        lock_ = lock;
+        PR_Lock(lock);
+    }
+    ~AutoSPSLock() { PR_Unlock(lock_); }
+#else
+    AutoSPSLock(PRLock *) {}
+#endif
+
+  private:
+    PRLock *lock_;
+};
+
+inline size_t
+SPSProfiler::stringsCount()
+{
+    AutoSPSLock lock(lock_);
+    return strings.count();
+}
+
+inline void
+SPSProfiler::stringsReset()
+{
+    AutoSPSLock lock(lock_);
+    strings.clear();
+}
+
 /*
  * This class is used in RunScript() to push the marker onto the sampling stack
  * that we're about to enter JS function calls. This is the only time in which a