From ef4a0edc383c8f8847ea5f2b582b693be6ea2643 Mon Sep 17 00:00:00 2001 From: Kannan Vijayan Date: Thu, 28 Mar 2013 14:50:17 -0400 Subject: [PATCH] Bug 853862 - Wrap ionStackLimit modifications with a lock. r=jandem,h4writer --- js/src/ion/Ion.cpp | 1 - js/src/jsapi.cpp | 24 ++++++++++++++++++++++-- js/src/jscntxt.cpp | 2 +- js/src/jscntxt.h | 33 ++++++++++++++++++++++++++++++++- js/src/vm/ForkJoin.cpp | 5 +++++ 5 files changed, 60 insertions(+), 5 deletions(-) diff --git a/js/src/ion/Ion.cpp b/js/src/ion/Ion.cpp index 7e3c1e4bedc9..7614258fcc93 100644 --- a/js/src/ion/Ion.cpp +++ b/js/src/ion/Ion.cpp @@ -398,7 +398,6 @@ IonActivation::IonActivation(JSContext *cx, StackFrame *fp) fp->setRunningInIon(); cx->mainThread().ionJSContext = cx; cx->mainThread().ionActivation = this; - cx->mainThread().ionStackLimit = cx->mainThread().nativeStackLimit; } IonActivation::~IonActivation() diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index dfd358e3dc29..5d206a9ee3c8 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -689,11 +689,14 @@ PerThreadData::PerThreadData(JSRuntime *runtime) ionTop(NULL), ionJSContext(NULL), ionStackLimit(0), +#ifdef JS_THREADSAFE + ionStackLimitLock_(NULL), +#endif ionActivation(NULL), asmJSActivationStack_(NULL), -# ifdef JS_THREADSAFE +#ifdef JS_THREADSAFE asmJSActivationStackLock_(NULL), -# endif +#endif suppressGC(0) {} @@ -701,6 +704,10 @@ bool PerThreadData::init() { #ifdef JS_THREADSAFE + ionStackLimitLock_ = PR_NewLock(); + if (!ionStackLimitLock_) + return false; + asmJSActivationStackLock_ = PR_NewLock(); if (!asmJSActivationStackLock_) return false; @@ -711,6 +718,9 @@ PerThreadData::init() PerThreadData::~PerThreadData() { #ifdef JS_THREADSAFE + if (ionStackLimitLock_) + PR_DestroyLock(ionStackLimitLock_); + if (asmJSActivationStackLock_) PR_DestroyLock(asmJSActivationStackLock_); #endif @@ -3032,6 +3042,16 @@ JS_SetNativeStackQuota(JSRuntime *rt, size_t stackSize) rt->mainThread.nativeStackLimit = rt->nativeStackBase - (stackSize - 1); } #endif + + // If there's no pending interrupt request set on the runtime's main thread's + // ionStackLimit, then update it so that it reflects the new nativeStacklimit. +#ifdef JS_ION + { + PerThreadData::IonStackLimitLock lock(rt->mainThread); + if (rt->mainThread.ionStackLimit != -1) + rt->mainThread.ionStackLimit = rt->mainThread.nativeStackLimit; + } +#endif } /************************************************************************/ diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index a989c537c13a..ab3f3bd6c10c 100644 --- a/js/src/jscntxt.cpp +++ b/js/src/jscntxt.cpp @@ -168,7 +168,7 @@ JSRuntime::triggerOperationCallback() * into a weird state where interrupt is stuck at 0 but ionStackLimit is * MAXADDR. */ - mainThread.ionStackLimit = -1; + mainThread.setIonStackLimit(-1); /* * Use JS_ATOMIC_SET in the hope that it ensures the write will become diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 5799bfaf6ffa..0520e34db5a2 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -483,6 +483,35 @@ class PerThreadData : public js::PerThreadDataFriendFields JSContext *ionJSContext; uintptr_t ionStackLimit; +# ifdef JS_THREADSAFE + /* + * Synchronizes setting of ionStackLimit so signals by triggerOperationCallback don't + * get lost. + */ + PRLock *ionStackLimitLock_; + + class IonStackLimitLock { + PerThreadData &data_; + public: + IonStackLimitLock(PerThreadData &data) : data_(data) { + JS_ASSERT(data_.ionStackLimitLock_); + PR_Lock(data_.ionStackLimitLock_); + } + ~IonStackLimitLock() { + JS_ASSERT(data_.ionStackLimitLock_); + PR_Unlock(data_.ionStackLimitLock_); + } + }; +#else + class IonStackLimitLock { + IonStackLimitLock(PerThreadData &data) {} + }; +# endif + void setIonStackLimit(uintptr_t limit) { + IonStackLimitLock lock(*this); + ionStackLimit = limit; + } + /* * This points to the most recent Ion activation running on the thread. */ @@ -1254,8 +1283,10 @@ struct JSRuntime : js::RuntimeFriendFields, bool jitHardening; + // Used to reset stack limit after a signaled interrupt (i.e. ionStackLimit_ = -1) + // has been noticed by Ion/Baseline. void resetIonStackLimit() { - mainThread.ionStackLimit = mainThread.nativeStackLimit; + mainThread.setIonStackLimit(mainThread.nativeStackLimit); } // Cache for ion::GetPcScript(). diff --git a/js/src/vm/ForkJoin.cpp b/js/src/vm/ForkJoin.cpp index 826314e5ee37..ee05e8700dd3 100644 --- a/js/src/vm/ForkJoin.cpp +++ b/js/src/vm/ForkJoin.cpp @@ -289,6 +289,8 @@ ForkJoinShared::executeFromWorker(uint32_t workerId, uintptr_t stackLimit) PerThreadData thisThread(cx_->runtime); TlsPerThreadData.set(&thisThread); + // Don't use setIonStackLimit() because that acquires the ionStackLimitLock, and the + // lock has not been initialized in these cases. thisThread.ionStackLimit = stackLimit; executePortion(&thisThread, workerId); TlsPerThreadData.set(NULL); @@ -549,6 +551,9 @@ ForkJoinSlice::triggerAbort() // In principle, we probably ought to set the ionStackLimit's for // the other threads too, but right now the various slice objects // are not on a central list so that's not possible. + + // Don't use setIonStackLimit() because that acquires the ionStackLimitLock, and the + // lock has not been initialized in these cases. perThreadData->ionStackLimit = -1; }