Bug 884676 - Convert JSPrincipals::refcount to Atomic<int32_t> in JS_THREADSAFE builds, and remove JS_ATOMIC_*. r=Waldo

caps/src/nsNullPrincipal.cpp

@@ -35,11 +35,11 @@ NS_IMPL_CI_INTERFACE_GETTER2(nsNullPrincipal,
                              nsIPrincipal,
                              nsISerializable)
 
-NS_IMETHODIMP_(nsrefcnt) 
+NS_IMETHODIMP_(nsrefcnt)
 nsNullPrincipal::AddRef()
 {
   NS_PRECONDITION(int32_t(refcount) >= 0, "illegal refcnt");
-  nsrefcnt count = PR_ATOMIC_INCREMENT(&refcount);
+  nsrefcnt count = ++refcount;
   NS_LOG_ADDREF(this, count, "nsNullPrincipal", sizeof(*this));
   return count;
 }
@@ -48,7 +48,7 @@ NS_IMETHODIMP_(nsrefcnt)
 nsNullPrincipal::Release()
 {
   NS_PRECONDITION(0 != refcount, "dup release");
-  nsrefcnt count = PR_ATOMIC_DECREMENT(&refcount);
+  nsrefcnt count = --refcount;
   NS_LOG_RELEASE(this, count, "nsNullPrincipal");
   if (count == 0) {
     delete this;

caps/src/nsPrincipal.cpp

@@ -51,7 +51,7 @@ nsBasePrincipal::AddRef()
 {
   NS_PRECONDITION(int32_t(refcount) >= 0, "illegal refcnt");
   // XXXcaa does this need to be threadsafe?  See bug 143559.
-  nsrefcnt count = PR_ATOMIC_INCREMENT(&refcount);
+  nsrefcnt count = ++refcount;
   NS_LOG_ADDREF(this, count, "nsBasePrincipal", sizeof(*this));
   return count;
 }
@@ -60,7 +60,7 @@ NS_IMETHODIMP_(nsrefcnt)
 nsBasePrincipal::Release()
 {
   NS_PRECONDITION(0 != refcount, "dup release");
-  nsrefcnt count = PR_ATOMIC_DECREMENT(&refcount);
+  nsrefcnt count = --refcount;
   NS_LOG_RELEASE(this, count, "nsBasePrincipal");
   if (count == 0) {
     delete this;

caps/src/nsSystemPrincipal.cpp

@@ -29,11 +29,11 @@ NS_IMPL_CI_INTERFACE_GETTER2(nsSystemPrincipal,
                              nsIPrincipal,
                              nsISerializable)
 
-NS_IMETHODIMP_(nsrefcnt) 
+NS_IMETHODIMP_(nsrefcnt)
 nsSystemPrincipal::AddRef()
 {
   NS_PRECONDITION(int32_t(refcount) >= 0, "illegal refcnt");
-  nsrefcnt count = PR_ATOMIC_INCREMENT(&refcount);
+  nsrefcnt count = ++refcount;
   NS_LOG_ADDREF(this, count, "nsSystemPrincipal", sizeof(*this));
   return count;
 }
@@ -42,7 +42,7 @@ NS_IMETHODIMP_(nsrefcnt)
 nsSystemPrincipal::Release()
 {
   NS_PRECONDITION(0 != refcount, "dup release");
-  nsrefcnt count = PR_ATOMIC_DECREMENT(&refcount);
+  nsrefcnt count = --refcount;
   NS_LOG_RELEASE(this, count, "nsSystemPrincipal");
   if (count == 0) {
     delete this;

dom/workers/Principal.cpp

@@ -9,21 +9,20 @@
 
 BEGIN_WORKERS_NAMESPACE
 
-namespace {
-
-JSPrincipals gPrincipal = {
-  1
-#ifdef DEBUG
-  , kJSPrincipalsDebugToken
-#endif
-};
-
-} // anonymous namespace
-
 JSPrincipals*
 GetWorkerPrincipal()
 {
-  return &gPrincipal;
+  static Atomic<uint32_t> sInitialized(0);
+  static JSPrincipals sPrincipal;
+
+  uint32_t isInitialized = sInitialized.exchange(1);
+  if (!isInitialized) {
+    sPrincipal.refcount = 1;
+#ifdef DEBUG
+    sPrincipal.debugToken = kJSPrincipalsDebugToken;
+#endif
+  }
+  return &sPrincipal;
 }
 
 END_WORKERS_NAMESPACE

js/src/frontend/TokenStream.cpp

@@ -287,7 +287,7 @@ TokenStream::TokenStream(ExclusiveContext *cx, const ReadOnlyCompileOptions &opt
 
     // The caller must ensure that a reference is held on the supplied principals
     // throughout compilation.
-    JS_ASSERT_IF(originPrincipals, originPrincipals->refcount);
+    JS_ASSERT_IF(originPrincipals, originPrincipals->refcount > 0);
 
     // Column numbers are computed as offsets from the current line's base, so the
     // initial line's base must be included in the buffer. linebase and userbuf

js/src/jsapi-tests/testChromeBuffer.cpp

@@ -6,9 +6,7 @@
 
 #include "jsapi-tests/tests.h"
 
-static JSPrincipals system_principals = {
-    1
-};
+static TestJSPrincipals system_principals(1);
 
 static const JSClass global_class = {
     "global",

js/src/jsapi-tests/testOriginPrincipals.cpp

@@ -6,8 +6,8 @@
 #include "jsapi-tests/tests.h"
 
 static JSPrincipals *sOriginPrincipalsInErrorReporter = nullptr;
-static JSPrincipals prin1 = { 1 };
-static JSPrincipals prin2 = { 1 };
+static TestJSPrincipals prin1(1);
+static TestJSPrincipals prin2(1);
 
 BEGIN_TEST(testOriginPrincipals)
 {

js/src/jsapi-tests/testXDR.cpp

@@ -76,10 +76,8 @@ FreezeThaw(JSContext *cx, JS::HandleObject funobj)
     return funobj2;
 }
 
-static JSPrincipals testPrincipals[] = {
-    { 1 },
-    { 1 },
-};
+static TestJSPrincipals testPrincipal0(1);
+static TestJSPrincipals testPrincipal1(1);
 
 BEGIN_TEST(testXDR_principals)
 {
@@ -88,21 +86,21 @@ BEGIN_TEST(testXDR_principals)
     for (int i = TEST_FIRST; i != TEST_END; ++i) {
         // Appease the new JSAPI assertions. The stuff being tested here is
         // going away anyway.
-        JS_SetCompartmentPrincipals(compartment, &testPrincipals[0]);
-        script = createScriptViaXDR(&testPrincipals[0], nullptr, i);
+        JS_SetCompartmentPrincipals(compartment, &testPrincipal0);
+        script = createScriptViaXDR(&testPrincipal0, nullptr, i);
         CHECK(script);
-        CHECK(JS_GetScriptPrincipals(script) == &testPrincipals[0]);
-        CHECK(JS_GetScriptOriginPrincipals(script) == &testPrincipals[0]);
+        CHECK(JS_GetScriptPrincipals(script) == &testPrincipal0);
+        CHECK(JS_GetScriptOriginPrincipals(script) == &testPrincipal0);
 
-        script = createScriptViaXDR(&testPrincipals[0], &testPrincipals[0], i);
+        script = createScriptViaXDR(&testPrincipal0, &testPrincipal0, i);
         CHECK(script);
-        CHECK(JS_GetScriptPrincipals(script) == &testPrincipals[0]);
-        CHECK(JS_GetScriptOriginPrincipals(script) == &testPrincipals[0]);
+        CHECK(JS_GetScriptPrincipals(script) == &testPrincipal0);
+        CHECK(JS_GetScriptOriginPrincipals(script) == &testPrincipal0);
 
-        script = createScriptViaXDR(&testPrincipals[0], &testPrincipals[1], i);
+        script = createScriptViaXDR(&testPrincipal0, &testPrincipal1, i);
         CHECK(script);
-        CHECK(JS_GetScriptPrincipals(script) == &testPrincipals[0]);
-        CHECK(JS_GetScriptOriginPrincipals(script) == &testPrincipals[1]);
+        CHECK(JS_GetScriptPrincipals(script) == &testPrincipal0);
+        CHECK(JS_GetScriptOriginPrincipals(script) == &testPrincipal1);
     }
 
     return true;

js/src/jsapi-tests/tests.h

@@ -400,4 +400,15 @@ class TempFile {
     }
 };
 
+// Just a wrapper around JSPrincipals that allows static construction.
+class TestJSPrincipals : public JSPrincipals
+{
+  public:
+    TestJSPrincipals(int rc = 0)
+      : JSPrincipals()
+    {
+        refcount = rc;
+    }
+};
+
 #endif /* jsapi_tests_tests_h */

js/src/jsapi.cpp

@@ -3864,13 +3864,13 @@ JS_CheckAccess(JSContext *cx, JSObject *objArg, jsid idArg, JSAccessMode mode,
 JS_PUBLIC_API(void)
 JS_HoldPrincipals(JSPrincipals *principals)
 {
-    JS_ATOMIC_INCREMENT(&principals->refcount);
+    ++principals->refcount;
 }
 
 JS_PUBLIC_API(void)
 JS_DropPrincipals(JSRuntime *rt, JSPrincipals *principals)
 {
-    int rc = JS_ATOMIC_DECREMENT(&principals->refcount);
+    int rc = --principals->refcount;
     if (rc == 0)
         rt->destroyPrincipals(principals);
 }

js/src/jsapi.h

@@ -9,6 +9,7 @@
 #ifndef jsapi_h
 #define jsapi_h
 
+#include "mozilla/Atomics.h"
 #include "mozilla/FloatingPoint.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/RangedPtr.h"
@@ -3155,7 +3156,11 @@ JS_SetReservedSlot(JSObject *obj, uint32_t index, jsval v);
  */
 struct JSPrincipals {
     /* Don't call "destroy"; use reference counting macros below. */
-    int refcount;
+#ifdef JS_THREADSAFE
+    mozilla::Atomic<int32_t> refcount;
+#else
+    int32_t refcount;
+#endif
 
 #ifdef DEBUG
     /* A helper to facilitate principals debugging. */

js/src/jslock.h

@@ -15,7 +15,6 @@
 
 #else /* JS_POSIX_NSPR */
 
-# include "pratom.h"
 # include "prcvar.h"
 # include "prinit.h"
 # include "prlock.h"
@@ -23,22 +22,12 @@
 
 #endif
 
-# define JS_ATOMIC_INCREMENT(p)      PR_ATOMIC_INCREMENT((int32_t *)(p))
-# define JS_ATOMIC_DECREMENT(p)      PR_ATOMIC_DECREMENT((int32_t *)(p))
-# define JS_ATOMIC_ADD(p,v)          PR_ATOMIC_ADD((int32_t *)(p), (int32_t)(v))
-# define JS_ATOMIC_SET(p,v)          PR_ATOMIC_SET((int32_t *)(p), (int32_t)(v))
-
 #else  /* JS_THREADSAFE */
 
 typedef struct PRThread PRThread;
 typedef struct PRCondVar PRCondVar;
 typedef struct PRLock PRLock;
 
-# define JS_ATOMIC_INCREMENT(p)      (++*(p))
-# define JS_ATOMIC_DECREMENT(p)      (--*(p))
-# define JS_ATOMIC_ADD(p,v)          (*(p) += (v))
-# define JS_ATOMIC_SET(p,v)          (*(p) = (v))
-
 #endif /* JS_THREADSAFE */
 
 #endif /* jslock_h */

js/src/shell/js.cpp

@@ -5652,9 +5652,6 @@ MaybeOverrideOutFileFromEnv(const char* const envVar,
     }
 }
 
-/* Set the initial counter to 1 so the principal will never be destroyed. */
-static const JSPrincipals shellTrustedPrincipals = { 1 };
-
 static bool
 CheckObjectAccess(JSContext *cx, HandleObject obj, HandleId id, JSAccessMode mode,
                   MutableHandleValue vp)
@@ -5885,6 +5882,10 @@ main(int argc, char **argv, char **envp)
         JS::DisableGenerationalGC(rt);
 #endif
 
+    /* Set the initial counter to 1 so the principal will never be destroyed. */
+    JSPrincipals shellTrustedPrincipals;
+    shellTrustedPrincipals.refcount = 1;
+
     JS_SetTrustedPrincipals(rt, &shellTrustedPrincipals);
     JS_SetSecurityCallbacks(rt, &securityCallbacks);
     JS_SetOperationCallback(rt, ShellOperationCallback);

js/src/vm/PosixNSPR.h

@@ -16,11 +16,6 @@
 #include <pthread.h>
 #include <stdint.h>
 
-#define PR_ATOMIC_INCREMENT(val) __sync_add_and_fetch(val, 1)
-#define PR_ATOMIC_DECREMENT(val) __sync_sub_and_fetch(val, 1)
-#define PR_ATOMIC_SET(val, newval) __sync_lock_test_and_set(val, newval)
-#define PR_ATOMIC_ADD(ptr, val) __sync_add_and_fetch(ptr, val)
-
 namespace nspr {
 class Thread;
 class Lock;

js/src/vm/Runtime.cpp

@@ -52,7 +52,11 @@ using JS::DoubleNaNValue;
 
 /* static */ ThreadLocal<PerThreadData*> js::TlsPerThreadData;
 
+#ifdef JS_THREADSAFE
 /* static */ Atomic<size_t> JSRuntime::liveRuntimesCount;
+#else
+/* static */ size_t JSRuntime::liveRuntimesCount;
+#endif
 
 const JSSecurityCallbacks js::NullSecurityCallbacks = { };
 

js/src/vm/Runtime.h

@@ -1605,7 +1605,11 @@ struct JSRuntime : public JS::shadow::Runtime,
     // their callee.
     js::Value            ionReturnOverride_;
 
+#ifdef JS_THREADSAFE
     static mozilla::Atomic<size_t> liveRuntimesCount;
+#else
+    static size_t liveRuntimesCount;
+#endif
 
   public:
     static bool hasLiveRuntimes() {