APIEntryShims need some love
authormhahnenberg@apple.com <mhahnenberg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Nov 2013 23:07:03 +0000 (23:07 +0000)
committermhahnenberg@apple.com <mhahnenberg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Nov 2013 23:07:03 +0000 (23:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=124540

Reviewed by Filip Pizlo.

We were missing them in key places which some other hacking revealed. These could have manifested as
race conditions for VMs being used in multithreaded environments.

* API/JSContext.mm:
(-[JSContext setException:]):
(-[JSContext wrapperForObjCObject:]):
(-[JSContext wrapperForJSObject:]):
* API/JSContextRef.cpp:
(JSContextGroupRelease):
(JSGlobalContextRelease):
* API/JSManagedValue.mm:
(-[JSManagedValue initWithValue:]):
(-[JSManagedValue value]):
* API/JSObjectRef.cpp:
(JSObjectIsFunction):
(JSObjectCopyPropertyNames):
* API/JSValue.mm:
(containerValueToObject):
* API/JSWrapperMap.mm:
(tryUnwrapObjcObject):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@159459 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/API/JSContext.mm
Source/JavaScriptCore/API/JSContextRef.cpp
Source/JavaScriptCore/API/JSManagedValue.mm
Source/JavaScriptCore/API/JSObjectRef.cpp
Source/JavaScriptCore/API/JSValue.mm
Source/JavaScriptCore/API/JSWrapperMap.mm
Source/JavaScriptCore/ChangeLog

index bc8fc1a..ef9e85b 100644 (file)
 
 - (void)setException:(JSValue *)value
 {
+    JSC::APIEntryShim entryShim(toJS(m_context));
     if (value)
         m_exception.set(toJS(m_context)->vm(), toJS(JSValueToObject(m_context, valueInternalValue(value), 0)));
     else
 
 - (JSValue *)wrapperForObjCObject:(id)object
 {
-    // Lock access to m_wrapperMap
-    JSC::JSLockHolder lock(toJS(m_context));
+    JSC::APIEntryShim entryShim(toJS(m_context));
     return [m_wrapperMap jsWrapperForObject:object];
 }
 
 - (JSValue *)wrapperForJSObject:(JSValueRef)value
 {
-    JSC::JSLockHolder lock(toJS(m_context));
+    JSC::APIEntryShim entryShim(toJS(m_context));
     return [m_wrapperMap objcWrapperForJSValueRef:value];
 }
 
index 2ed8241..108fd59 100644 (file)
@@ -68,16 +68,9 @@ JSContextGroupRef JSContextGroupRetain(JSContextGroupRef group)
 
 void JSContextGroupRelease(JSContextGroupRef group)
 {
-    IdentifierTable* savedIdentifierTable;
     VM& vm = *toJS(group);
-
-    {
-        JSLockHolder lock(vm);
-        savedIdentifierTable = wtfThreadData().setCurrentIdentifierTable(vm.identifierTable);
-        vm.deref();
-    }
-
-    wtfThreadData().setCurrentIdentifierTable(savedIdentifierTable);
+    APIEntryShim entryShim(&vm);
+    vm.deref();
 }
 
 static bool internalScriptTimeoutCallback(ExecState* exec, void* callbackPtr, void* callbackData)
@@ -165,7 +158,7 @@ void JSGlobalContextRelease(JSGlobalContextRef ctx)
     IdentifierTable* savedIdentifierTable;
     ExecState* exec = toJS(ctx);
     {
-        JSLockHolder lock(exec);
+        APIEntryShim entryShim(exec);
 
         VM& vm = exec->vm();
         savedIdentifierTable = wtfThreadData().setCurrentIdentifierTable(vm.identifierTable);
index 6013303..4616014 100644 (file)
@@ -30,6 +30,7 @@
 #if JSC_OBJC_API_ENABLED
 
 #import "APICast.h"
+#import "APIShims.h"
 #import "Heap.h"
 #import "JSContextInternal.h"
 #import "JSValueInternal.h"
@@ -191,6 +192,7 @@ private:
         return self;
 
     JSC::ExecState* exec = toJS([value.context JSGlobalContextRef]);
+    JSC::APIEntryShim entryShim(exec);
     JSC::JSGlobalObject* globalObject = exec->lexicalGlobalObject();
     JSC::Weak<JSC::JSGlobalObject> weak(globalObject, managedValueHandleOwner(), self);
     m_globalObject.swap(weak);
@@ -212,6 +214,7 @@ private:
     if (m_weakValue.isClear())
         return nil;
     JSC::ExecState* exec = m_globalObject->globalExec();
+    JSC::APIEntryShim entryShim(exec);
     JSContext *context = [JSContext contextWithJSGlobalContextRef:toGlobalRef(exec)];
     JSC::JSValue value;
     if (m_weakValue.isPrimitive())
index 52e3a30..c769514 100644 (file)
@@ -506,10 +506,11 @@ bool JSObjectDeletePrivateProperty(JSContextRef ctx, JSObjectRef object, JSStrin
     return false;
 }
 
-bool JSObjectIsFunction(JSContextRef, JSObjectRef object)
+bool JSObjectIsFunction(JSContextRef ctx, JSObjectRef object)
 {
     if (!object)
         return false;
+    APIEntryShim entryShim(toJS(ctx));
     CallData callData;
     JSCell* cell = toJS(object);
     return cell->methodTable()->getCallData(cell, callData) != CallTypeNone;
@@ -605,12 +606,12 @@ JSPropertyNameArrayRef JSObjectCopyPropertyNames(JSContextRef ctx, JSObjectRef o
         ASSERT_NOT_REACHED();
         return 0;
     }
-    JSObject* jsObject = toJS(object);
     ExecState* exec = toJS(ctx);
     APIEntryShim entryShim(exec);
 
     VM* vm = &exec->vm();
 
+    JSObject* jsObject = toJS(object);
     JSPropertyNameArrayRef propertyNames = new OpaqueJSPropertyNameArray(vm);
     PropertyNameArray array(vm);
     jsObject->methodTable()->getPropertyNames(jsObject, exec, array, ExcludeDontEnumProperties);
index 95c129e..b4a50a6 100644 (file)
@@ -692,6 +692,8 @@ static id containerValueToObject(JSGlobalContextRef context, JSContainerConverto
             ASSERT([current.objc isKindOfClass:[NSMutableDictionary class]]);
             NSMutableDictionary *dictionary = (NSMutableDictionary *)current.objc;
 
+            JSC::APIEntryShim entryShim(toJS(context));
+
             JSPropertyNameArrayRef propertyNameArray = JSObjectCopyPropertyNames(context, js);
             size_t length = JSPropertyNameArrayGetCount(propertyNameArray);
 
index 2f62fcc..5e4613f 100644 (file)
@@ -603,6 +603,7 @@ id tryUnwrapObjcObject(JSGlobalContextRef context, JSValueRef value)
     JSValueRef exception = 0;
     JSObjectRef object = JSValueToObject(context, value, &exception);
     ASSERT(!exception);
+    JSC::APIEntryShim entryShim(toJS(context));
     if (toJS(object)->inherits(JSC::JSCallbackObject<JSC::JSAPIWrapperObject>::info()))
         return (id)JSC::jsCast<JSC::JSAPIWrapperObject*>(toJS(object))->wrappedObject();
     if (id target = tryUnwrapConstructor(object))
index 4d415dc..c726ef4 100644 (file)
@@ -1,3 +1,31 @@
+2013-11-18  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        APIEntryShims need some love
+        https://bugs.webkit.org/show_bug.cgi?id=124540
+
+        Reviewed by Filip Pizlo.
+
+        We were missing them in key places which some other hacking revealed. These could have manifested as
+        race conditions for VMs being used in multithreaded environments.
+
+        * API/JSContext.mm:
+        (-[JSContext setException:]):
+        (-[JSContext wrapperForObjCObject:]):
+        (-[JSContext wrapperForJSObject:]):
+        * API/JSContextRef.cpp:
+        (JSContextGroupRelease):
+        (JSGlobalContextRelease):
+        * API/JSManagedValue.mm:
+        (-[JSManagedValue initWithValue:]):
+        (-[JSManagedValue value]):
+        * API/JSObjectRef.cpp:
+        (JSObjectIsFunction):
+        (JSObjectCopyPropertyNames):
+        * API/JSValue.mm:
+        (containerValueToObject):
+        * API/JSWrapperMap.mm:
+        (tryUnwrapObjcObject):
+
 2013-11-18  Filip Pizlo  <fpizlo@apple.com>
 
         Allow the FTL debug dumps to include the new size field