Reviewed by Darin
Fix for <rdar://
5747529> - ObjC Exception can cause JSLock to never be released
Test: platform/mac/plugins/webScriptObject-exception-deadlock.html
* bindings/objc/WebScriptObject.mm:
(-[WebScriptObject valueForKey:]): The line `resultObj = [super valueForKey:key]; // defaults to throwing an exception`
says it all - it throws an exception. This method also happens to hold the JSLock. Problematically, when the exeception
is thrown and the method exited, the JSLock is never released. Fix that without otherwise changing behavior by holding the
JSLock in two individual scopes - Right before the exception and right after.
WebKitTools:
Changes by Geoff Garen, Reviewed by Darin
Fix for <rdar://
5747529> - ObjC Exception can cause JSLock to never be released
DRT changes for test: platform/mac/plugins/webScriptObject-exception-deadlock.html
[WebScriptObject valueForKey:] might throw an exception, and previously might have "leaked" the global JSLock
This test calls valueForKey, then runs some arbitrary Javascript on a 2ndary thread. If the lock has leaked,
this series of method calls will deadlock. If things are good, it will complete successfully.
* DumpRenderTree/mac/ObjCController.m:
(runJavaScriptThread):
(+[ObjCController isSelectorExcludedFromWebScript:]):
(+[ObjCController webScriptNameForSelector:]):
(-[ObjCController testValueForKey]):
LayoutTests:
Reviewed by Darin
Fix for <rdar://
5747529> - ObjC Exception can cause JSLock to never be released
* platform/mac-tiger/Skipped: Removed 2 hanging tests that now don't hang
* platform/mac/plugins/webScriptObject-exception-deadlock-expected.txt: Added.
* platform/mac/plugins/webScriptObject-exception-deadlock.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@30395
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2008-02-18 Brady Eidson <beidson@apple.com>
+
+ Reviewed by Darin
+
+ Fix for <rdar://5747529> - ObjC Exception can cause JSLock to never be released
+
+ * platform/mac-tiger/Skipped: Removed 2 hanging tests that now don't hang
+ * platform/mac/plugins/webScriptObject-exception-deadlock-expected.txt: Added.
+ * platform/mac/plugins/webScriptObject-exception-deadlock.html: Added.
+
2008-02-18 Darin Adler <darin@apple.com>
Reviewed by Sam.
# For this test to work on Tiger we'd need a new Foundation
http/tests/xmlhttprequest/small-chunks-response-text.html
-# Storage tests failing only on Tiger - <rdar://problem/5747529>
-storage/quota-tracking.html
-storage/success-callback.html
-
# On Tiger, WebKit does not override the MIME type returned by NSHTTPURLResponse
http/tests/loading/text-content-type-with-binary-extension.html
--- /dev/null
+This test checks for an exception in [WebScriptObject valueForKey:(NSString *)key] That method is liable to throw an exception which caused the JSLock to never be released, leading to a possible deadlock. If this test doesn't hang, it passed.
--- /dev/null
+<html>
+<script>
+
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ objCController.storeWebScriptObject(window);
+ objCController.testValueForKey();
+}
+
+</script>
+<body>
+This test checks for an exception in [WebScriptObject valueForKey:(NSString *)key]
+That method is liable to throw an exception which caused the JSLock to never be released, leading to a possible deadlock.
+If this test doesn't hang, it passed.
+</body>
+</html>
+2008-02-18 Brady Eidson <beidson@apple.com>
+
+ Reviewed by Darin
+
+ Fix for <rdar://5747529> - ObjC Exception can cause JSLock to never be released
+
+ Test: platform/mac/plugins/webScriptObject-exception-deadlock.html
+
+ * bindings/objc/WebScriptObject.mm:
+ (-[WebScriptObject valueForKey:]): The line `resultObj = [super valueForKey:key]; // defaults to throwing an exception`
+ says it all - it throws an exception. This method also happens to hold the JSLock. Problematically, when the exeception
+ is thrown and the method exited, the JSLock is never released. Fix that without otherwise changing behavior by holding the
+ JSLock in two individual scopes - Right before the exception and right after.
+
2008-02-18 Darin Adler <darin@apple.com>
Reviewed by Sam.
ExecState* exec = [self _rootObject]->globalObject()->globalExec();
ASSERT(!exec->hadException());
- JSLock lock;
- JSValue *result = [self _imp]->get(exec, String(key));
-
- if (exec->hadException()) {
- LOG_EXCEPTION(exec);
- result = jsUndefined();
- exec->clearException();
- }
+ id resultObj;
+ {
+ // Need to scope this lock to ensure that we release the lock before calling
+ // [super valueForKey:key] which might throw an exception and bypass the JSLock destructor,
+ // leaving the lock permanently held
+ JSLock lock;
+
+ JSValue *result = [self _imp]->get(exec, String(key));
+
+ if (exec->hadException()) {
+ LOG_EXCEPTION(exec);
+ result = jsUndefined();
+ exec->clearException();
+ }
- id resultObj = [WebScriptObject _convertValueToObjcValue:result originRootObject:[self _originRootObject] rootObject:[self _rootObject]];
+ resultObj = [WebScriptObject _convertValueToObjcValue:result originRootObject:[self _originRootObject] rootObject:[self _rootObject]];
+ }
+
if ([resultObj isKindOfClass:[WebUndefined class]])
resultObj = [super valueForKey:key]; // defaults to throwing an exception
+ JSLock lock;
_didExecute(self);
return resultObj;
+2008-02-18 Brady Eidson <beidson@apple.com>
+
+ Changes by Geoff Garen, Reviewed by Darin
+
+ Fix for <rdar://5747529> - ObjC Exception can cause JSLock to never be released
+
+ DRT changes for test: platform/mac/plugins/webScriptObject-exception-deadlock.html
+
+ [WebScriptObject valueForKey:] might throw an exception, and previously might have "leaked" the global JSLock
+ This test calls valueForKey, then runs some arbitrary Javascript on a 2ndary thread. If the lock has leaked,
+ this series of method calls will deadlock. If things are good, it will complete successfully.
+
+ * DumpRenderTree/mac/ObjCController.m:
+ (runJavaScriptThread):
+ (+[ObjCController isSelectorExcludedFromWebScript:]):
+ (+[ObjCController webScriptNameForSelector:]):
+ (-[ObjCController testValueForKey]):
+
2008-02-18 Matt Lilek <webkit@mattlilek.com>
Reviewed by Adam.
#import "ObjCController.h"
+#import <JavaScriptCore/JavaScriptCore.h>
#import <WebKit/DOMAbstractView.h>
#import <WebKit/WebScriptObject.h>
#import <WebKit/WebView.h>
+#import <pthread.h>
#import <wtf/Assertions.h>
+static void* runJavaScriptThread(void* arg)
+{
+ JSGlobalContextRef ctx = JSGlobalContextCreate(0);
+ JSStringRef scriptRef = JSStringCreateWithUTF8CString("'Hello World!'");
+
+ JSValueRef exception = 0;
+ JSEvaluateScript(ctx, scriptRef, 0, 0, 0, &exception);
+ ASSERT(!exception);
+
+ JSGlobalContextRelease(ctx);
+ JSStringRelease(scriptRef);
+
+ return 0;
+}
+
@implementation ObjCController
+ (BOOL)isSelectorExcludedFromWebScript:(SEL)aSelector
|| aSelector == @selector(testWrapperRoundTripping:)
|| aSelector == @selector(accessStoredWebScriptObject)
|| aSelector == @selector(storeWebScriptObject:)
+ || aSelector == @selector(testValueForKey)
)
return NO;
return YES;
return @"testWrapperRoundTripping";
if (aSelector == @selector(storeWebScriptObject:))
return @"storeWebScriptObject";
+ if (aSelector == @selector(testValueForKey))
+ return @"testValueForKey";
return nil;
}
return num;
}
+- (void)testValueForKey
+{
+ ASSERT(storedWebScriptObject);
+
+ @try {
+ [storedWebScriptObject valueForKey:@"ThisKeyDoesNotExist"];
+ } @catch (NSException *e) {
+ }
+
+ pthread_t pthread;
+ pthread_create(&pthread, 0, &runJavaScriptThread, 0);
+ pthread_join(pthread, 0);
+}
+
- (BOOL)testWrapperRoundTripping:(WebScriptObject *)webScriptObject
{
JSObjectRef jsObject = [webScriptObject JSObject];
git://git.webkit.org / WebKit-https.git / commitdiff