DelayedReleaseScope drops locks during GC which can cause a thread switch and code...
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Mar 2015 05:33:37 +0000 (05:33 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Mar 2015 05:33:37 +0000 (05:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141275

Reviewed by Geoffrey Garen.

The original issue is that the CodeCache uses an unsafe method to add new UnlinkedCodeBlocks.
It basically adds a null UnlinkedCodeBlock if there isn't a cached entry and then later
updates the null entry to the result of the compilation.  If during that compilation and
related processing we need to garbage collect, the DelayedReleaseScope would drop locks
possibly allowing another thread to try to get the same source out of the CodeCache.
This second thread would find the null entry and crash.  The fix is to move the processing of
DelayedReleaseScope to when we drop locks and not drop locks during GC.  That was done in
the original patch with the new function releaseDelayedReleasedObjects().

Updated releaseDelayedReleasedObjects() so that objects are released with all locks
dropped.  Now its processing follows these steps
    Increment recursion counter and do recursion check and exit if recursing
    While there are objects to release
        ASSERT that lock is held by current thread
        Take all items from delayed release Vector and put into temporary Vector
        Release API lock
        Release and clear items from temporary vector
        Reaquire API lock
This meets the requirement that we release while the API lock is released and it is
safer processing of the delayed release Vector.

Added new regression test to testapi.

Also added comment describing how recursion into releaseDelayedReleasedObjects() is
prevented.

* API/tests/Regress141275.h: Added.
* API/tests/Regress141275.mm: Added.
(+[JSTEvaluatorTask evaluatorTaskWithEvaluateBlock:completionHandler:]):
(-[JSTEvaluator init]):
(-[JSTEvaluator initWithScript:]):
(-[JSTEvaluator _accessPendingTasksWithBlock:]):
(-[JSTEvaluator insertSignPostWithCompletion:]):
(-[JSTEvaluator evaluateScript:completion:]):
(-[JSTEvaluator evaluateBlock:completion:]):
(-[JSTEvaluator waitForTasksDoneAndReportResults]):
(__JSTRunLoopSourceScheduleCallBack):
(__JSTRunLoopSourcePerformCallBack):
(__JSTRunLoopSourceCancelCallBack):
(-[JSTEvaluator _jsThreadMain]):
(-[JSTEvaluator _sourceScheduledOnRunLoop:]):
(-[JSTEvaluator _setupEvaluatorThreadContextIfNeeded]):
(-[JSTEvaluator _callCompletionHandler:ifNeededWithError:]):
(-[JSTEvaluator _sourcePerform]):
(-[JSTEvaluator _sourceCanceledOnRunLoop:]):
(runRegress141275):
* API/tests/testapi.mm:
(testObjectiveCAPI):
* JavaScriptCore.xcodeproj/project.pbxproj:
* heap/Heap.cpp:
(JSC::Heap::releaseDelayedReleasedObjects):
* runtime/JSLock.cpp:
(JSC::JSLock::unlock):

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

Source/JavaScriptCore/API/tests/Regress141275.h [new file with mode: 0644]
Source/JavaScriptCore/API/tests/Regress141275.mm [new file with mode: 0644]
Source/JavaScriptCore/API/tests/testapi.mm
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/runtime/JSLock.cpp

diff --git a/Source/JavaScriptCore/API/tests/Regress141275.h b/Source/JavaScriptCore/API/tests/Regress141275.h
new file mode 100644 (file)
index 0000000..bf3492a
--- /dev/null
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#import <Foundation/Foundation.h>
+#import <JavaScriptCore/JavaScriptCore.h>
+
+#if JSC_OBJC_API_ENABLED
+
+void runRegress141275();
+
+#endif // JSC_OBJC_API_ENABLED
+
diff --git a/Source/JavaScriptCore/API/tests/Regress141275.mm b/Source/JavaScriptCore/API/tests/Regress141275.mm
new file mode 100644 (file)
index 0000000..18e186a
--- /dev/null
@@ -0,0 +1,388 @@
+/*
+ * Copyright (C) 2015 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#import "config.h"
+#import "Regress141275.h"
+
+#import <Foundation/Foundation.h>
+#import <objc/objc.h>
+#import <objc/runtime.h>
+
+#if JSC_OBJC_API_ENABLED
+
+extern "C" void JSSynchronousGarbageCollectForDebugging(JSContextRef);
+
+extern int failed;
+
+static const NSUInteger scriptToEvaluate = 50;
+
+@interface JSTEvaluator : NSObject
+- (instancetype)initWithScript:(NSString*)script;
+
+- (void)insertSignPostWithCompletion:(void(^)(NSError* error))completionHandler;
+
+- (void)evaluateScript:(NSString*)script completion:(void(^)(NSError* error))completionHandler;
+- (void)evaluateBlock:(void(^)(JSContext* context))evaluationBlock completion:(void(^)(NSError* error))completionHandler;
+
+- (void)waitForTasksDoneAndReportResults;
+@end
+
+
+static const NSString* JSTEvaluatorThreadContextKey = @"JSTEvaluatorThreadContextKey";
+
+/*
+ * A JSTEvaluatorThreadContext is kept in the thread dictionary of threads used by JSEvaluator.
+ *
+ * This includes the run loop thread, and any threads used by _jsSourcePerformQueue to execute a task.
+ */
+@interface JSTEvaluatorThreadContext : NSObject
+@property (weak) JSTEvaluator* evaluator;
+@property (strong) JSContext* jsContext;
+@end
+
+@implementation JSTEvaluatorThreadContext
+@end
+
+
+/*!
+ * A JSTEvaluatorTask is a single task to be executed.
+ *
+ * JSTEvaluator keeps a list of pending tasks. The run loop thread is repsonsible for feeding pending tasks to the _jsSourcePerformQueue, while respecting sign posts.
+ */
+@interface JSTEvaluatorTask : NSObject
+
+@property (nonatomic, copy) void (^evaluateBlock)(JSContext* jsContext);
+@property (nonatomic, copy) void (^completionHandler)(NSError* error);
+@property (nonatomic, copy) NSError* error;
+
++ (instancetype)evaluatorTaskWithEvaluateBlock:(void (^)(JSContext*))block completionHandler:(void (^)(NSError* error))completionBlock;
+
+@end
+
+@implementation JSTEvaluatorTask
+
++ (instancetype)evaluatorTaskWithEvaluateBlock:(void (^)(JSContext*))evaluationBlock completionHandler:(void (^)(NSError* error))completionHandler
+{
+    JSTEvaluatorTask* task = [self new];
+    task.evaluateBlock = evaluationBlock;
+    task.completionHandler = completionHandler;
+    return task;
+}
+
+@end
+
+@implementation JSTEvaluator {
+    dispatch_queue_t _jsSourcePerformQueue;
+    dispatch_semaphore_t _allScriptsDone;
+    CFRunLoopRef _jsThreadRunLoop;
+    CFRunLoopSourceRef _jsThreadRunLoopSource;
+    JSContext* _jsContext;
+    NSMutableArray* __pendingTasks;
+}
+
+- (instancetype)init
+{
+    self = [super init];
+    if (self) {
+        _jsSourcePerformQueue = dispatch_queue_create("JSTEval", DISPATCH_QUEUE_CONCURRENT);
+
+        _allScriptsDone = dispatch_semaphore_create(0);
+
+        _jsContext = [JSContext new];
+        _jsContext.name = @"JSTEval";
+        __pendingTasks = [NSMutableArray new];
+
+        NSThread* jsThread = [[NSThread alloc] initWithTarget:self selector:@selector(_jsThreadMain) object:nil];
+        [jsThread setName:@"JSTEval"];
+        [jsThread start];
+
+    }
+    return self;
+}
+
+- (instancetype)initWithScript:(NSString*)script
+{
+    self = [self init];
+    if (self) {
+        __block NSError* scriptError = nil;
+        dispatch_semaphore_t dsema = dispatch_semaphore_create(0);
+        [self evaluateScript:script
+            completion:^(NSError* error) {
+                scriptError = error;
+                dispatch_semaphore_signal(dsema);
+            }];
+        dispatch_semaphore_wait(dsema, DISPATCH_TIME_FOREVER);
+    }
+    return self;
+}
+
+- (void)_accessPendingTasksWithBlock:(void(^)(NSMutableArray* pendingTasks))block
+{
+    @synchronized(self) {
+        block(__pendingTasks);
+        if (__pendingTasks.count > 0) {
+            if (_jsThreadRunLoop && _jsThreadRunLoopSource) {
+                CFRunLoopSourceSignal(_jsThreadRunLoopSource);
+                CFRunLoopWakeUp(_jsThreadRunLoop);
+            }
+        }
+    }
+}
+
+- (void)insertSignPostWithCompletion:(void(^)(NSError* error))completionHandler
+{
+    [self _accessPendingTasksWithBlock:^(NSMutableArray* pendingTasks) {
+        JSTEvaluatorTask* task = [JSTEvaluatorTask evaluatorTaskWithEvaluateBlock:nil
+            completionHandler:completionHandler];
+
+        [pendingTasks addObject:task];
+    }];
+}
+
+- (void)evaluateScript:(NSString*)script completion:(void(^)(NSError* error))completionHandler
+{
+    [self evaluateBlock:^(JSContext* context) {
+        [context evaluateScript:script];
+    } completion:completionHandler];
+}
+
+- (void)evaluateBlock:(void(^)(JSContext* context))evaluationBlock completion:(void(^)(NSError* error))completionHandler
+{
+    NSParameterAssert(evaluationBlock != nil);
+    [self _accessPendingTasksWithBlock:^(NSMutableArray* pendingTasks) {
+        JSTEvaluatorTask* task = [JSTEvaluatorTask evaluatorTaskWithEvaluateBlock:evaluationBlock
+            completionHandler:completionHandler];
+
+        [pendingTasks addObject:task];
+    }];
+}
+
+- (void)waitForTasksDoneAndReportResults
+{
+    NSString* passFailString = @"PASSED";
+
+    if (!dispatch_semaphore_wait(_allScriptsDone, dispatch_time(DISPATCH_TIME_NOW, 30 * NSEC_PER_SEC))) {
+        int totalScriptsRun = [_jsContext[@"counter"] toInt32];
+
+        if (totalScriptsRun != scriptToEvaluate) {
+            passFailString = @"FAILED";
+            failed = 1;
+        }
+
+        NSLog(@"  Ran a total of %d scripts: %@", totalScriptsRun, passFailString);
+    } else {
+        passFailString = @"FAILED";
+        failed = 1;
+        NSLog(@"  Error, timeout waiting for all tasks to complete: %@", passFailString);
+    }
+}
+
+static void __JSTRunLoopSourceScheduleCallBack(void* info, CFRunLoopRef rl, CFStringRef)
+{
+    @autoreleasepool {
+        [(__bridge JSTEvaluator*)info _sourceScheduledOnRunLoop:rl];
+    }
+}
+
+static void __JSTRunLoopSourcePerformCallBack(void* info )
+{
+    @autoreleasepool {
+        [(__bridge JSTEvaluator*)info _sourcePerform];
+    }
+}
+
+static void __JSTRunLoopSourceCancelCallBack(void* info, CFRunLoopRef rl, CFStringRef)
+{
+    @autoreleasepool {
+        [(__bridge JSTEvaluator*)info _sourceCanceledOnRunLoop:rl];
+    }
+}
+
+- (void)_jsThreadMain
+{
+    @autoreleasepool {
+        const CFIndex kRunLoopSourceContextVersion = 0;
+        CFRunLoopSourceContext sourceContext = {
+            kRunLoopSourceContextVersion, (__bridge void*)(self),
+            NULL, NULL, NULL, NULL, NULL,
+            __JSTRunLoopSourceScheduleCallBack,
+            __JSTRunLoopSourceCancelCallBack,
+            __JSTRunLoopSourcePerformCallBack
+        };
+
+        @synchronized(self) {
+            _jsThreadRunLoop = CFRunLoopGetCurrent();
+            CFRetain(_jsThreadRunLoop);
+
+            _jsThreadRunLoopSource = CFRunLoopSourceCreate(kCFAllocatorDefault, 0, &sourceContext);
+            CFRunLoopAddSource(_jsThreadRunLoop, _jsThreadRunLoopSource, kCFRunLoopDefaultMode);
+        }
+
+        CFRunLoopRun();
+
+        @synchronized(self) {
+            NSMutableDictionary* threadDict = [[NSThread currentThread] threadDictionary];
+            [threadDict removeObjectForKey:threadDict[JSTEvaluatorThreadContextKey]];
+
+            CFRelease(_jsThreadRunLoopSource);
+            _jsThreadRunLoopSource = NULL;
+
+            CFRelease(_jsThreadRunLoop);
+            _jsThreadRunLoop = NULL;
+
+            __pendingTasks = nil;
+        }
+    }
+}
+
+- (void)_sourceScheduledOnRunLoop:(CFRunLoopRef)runLoop
+{
+    UNUSED_PARAM(runLoop);
+    assert([[[NSThread currentThread] name] isEqualToString:@"JSTEval"]);
+
+    // Wake up the run loop in case requests were submitted prior to the
+    // run loop & run loop source getting created.
+    CFRunLoopSourceSignal(_jsThreadRunLoopSource);
+    CFRunLoopWakeUp(_jsThreadRunLoop);
+}
+
+- (void)_setupEvaluatorThreadContextIfNeeded
+{
+    NSMutableDictionary* threadDict = [[NSThread currentThread] threadDictionary];
+    JSTEvaluatorThreadContext* context = threadDict[JSTEvaluatorThreadContextKey];
+    // The evaluator may be other evualuator, or nil if this thread has not been used before. Eaither way take ownership.
+    if (context.evaluator != self) {
+        context = [JSTEvaluatorThreadContext new];
+        context.evaluator = self;
+        threadDict[JSTEvaluatorThreadContextKey] = context;
+    }
+}
+
+- (void)_callCompletionHandler:(void(^)(NSError* error))completionHandler ifNeededWithError:(NSError*)error
+{
+    if (completionHandler) {
+        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+            completionHandler(error);
+        });
+    }
+}
+
+- (void)_sourcePerform
+{
+    assert([[[NSThread currentThread] name] isEqualToString:@"JSTEval"]);
+
+    __block NSArray* tasks = nil;
+    [self _accessPendingTasksWithBlock:^(NSMutableArray* pendingTasks) {
+        // No signpost, take all tasks.
+        tasks = [pendingTasks copy];
+        [pendingTasks removeAllObjects];
+    }];
+
+    if (tasks.count > 0) {
+        for (JSTEvaluatorTask* task in tasks) {
+            dispatch_block_t block = ^{
+                NSError* error = nil;
+                if (task.evaluateBlock) {
+                    [self _setupEvaluatorThreadContextIfNeeded];
+                    task.evaluateBlock(_jsContext);
+                    if (_jsContext.exception) {
+                        NSLog(@"Did fail on JSContext: %@", _jsContext.name);
+                        NSDictionary* userInfo = @{ NSLocalizedDescriptionKey : [_jsContext.exception[@"message"] toString] };
+                        error = [NSError errorWithDomain:@"JSTEvaluator" code:1 userInfo:userInfo];
+                        _jsContext.exception = nil;
+                    }
+                }
+                [self _callCompletionHandler:task.completionHandler ifNeededWithError:error];
+            };
+
+            if (task.evaluateBlock)
+                dispatch_async(_jsSourcePerformQueue, block);
+            else
+                dispatch_barrier_async(_jsSourcePerformQueue, block);
+        }
+
+        dispatch_barrier_sync(_jsSourcePerformQueue, ^{
+            if ([_jsContext[@"counter"] toInt32] == scriptToEvaluate)
+                dispatch_semaphore_signal(_allScriptsDone);
+        });
+    }
+}
+
+- (void)_sourceCanceledOnRunLoop:(CFRunLoopRef)runLoop
+{
+    UNUSED_PARAM(runLoop);
+    assert([[[NSThread currentThread] name] isEqualToString:@"JSTEval"]);
+
+    @synchronized(self) {
+        assert(_jsThreadRunLoop);
+        assert(_jsThreadRunLoopSource);
+
+        CFRunLoopRemoveSource(_jsThreadRunLoop, _jsThreadRunLoopSource, kCFRunLoopDefaultMode);
+        CFRunLoopStop(_jsThreadRunLoop);
+    }
+}
+
+@end
+
+void runRegress141275()
+{
+    // Test that we can execute the same script from multiple threads with a shared context.
+    // See <https://webkit.org/b/141275>
+    NSLog(@"TEST: Testing multiple threads executing the same script with a shared context");
+
+    @autoreleasepool {
+        JSTEvaluator* evaluator = [[JSTEvaluator alloc] initWithScript:@"this['counter'] = 0;"];
+
+        void (^showErrorIfNeeded)(NSError* error) = ^(NSError* error) {
+            if (error) {
+                dispatch_async(dispatch_get_main_queue(), ^{
+                    NSLog(@"Error: %@", error);
+                });
+            }
+        };
+
+        [evaluator evaluateBlock:^(JSContext* context) {
+            JSSynchronousGarbageCollectForDebugging([context JSGlobalContextRef]);
+        } completion:showErrorIfNeeded];
+
+        [evaluator evaluateBlock:^(JSContext* context) {
+            context[@"wait"] = ^{
+                [NSThread sleepForTimeInterval:0.01];
+            };
+        } completion:^(NSError* error) {
+            if (error) {
+                dispatch_async(dispatch_get_main_queue(), ^{
+                    NSLog(@"Error: %@", error);
+                });
+            }
+            for (unsigned i = 0; i < scriptToEvaluate; i++)
+                [evaluator evaluateScript:@"this['counter']++; this['wait']();" completion:showErrorIfNeeded];
+        }];
+
+        [evaluator waitForTasksDoneAndReportResults];
+    }
+}
+
+#endif // JSC_OBJC_API_ENABLED
index 724867c6400e5876fd479234dd80c7d7e0e701c0..9aadcd09743e85411ec91772eb1d6f635eb3c17d 100644 (file)
@@ -28,6 +28,7 @@
 #import "CurrentThisInsideBlockGetterTest.h"
 #import "DateTests.h"
 #import "JSExportTests.h"
+#import "Regress141275.h"
 #import "Regress141809.h"
 
 #import <pthread.h>
@@ -486,7 +487,6 @@ static void* threadMain(void* contextPtr)
 void testObjectiveCAPI()
 {
     NSLog(@"Testing Objective-C API");
-
     @autoreleasepool {
         JSVirtualMachine* vm = [[JSVirtualMachine alloc] init];
         JSContext* context = [[JSContext alloc] initWithVirtualMachine:vm];
@@ -1386,6 +1386,7 @@ void testObjectiveCAPI()
     currentThisInsideBlockGetterTest();
     runDateTests();
     runJSExportTests();
+    runRegress141275();
     runRegress141809();
 }
 
index afd3899133fee0c484894a850490bd2eab17de3f..2450ceb1caad357b5d00034cbf07d8126eedb178 100644 (file)
@@ -1,3 +1,64 @@
+2015-03-03  Michael Saboff  <msaboff@apple.com>
+
+        DelayedReleaseScope drops locks during GC which can cause a thread switch and code reentry
+        https://bugs.webkit.org/show_bug.cgi?id=141275
+
+        Reviewed by Geoffrey Garen.
+
+        The original issue is that the CodeCache uses an unsafe method to add new UnlinkedCodeBlocks.
+        It basically adds a null UnlinkedCodeBlock if there isn't a cached entry and then later
+        updates the null entry to the result of the compilation.  If during that compilation and
+        related processing we need to garbage collect, the DelayedReleaseScope would drop locks
+        possibly allowing another thread to try to get the same source out of the CodeCache.
+        This second thread would find the null entry and crash.  The fix is to move the processing of
+        DelayedReleaseScope to when we drop locks and not drop locks during GC.  That was done in
+        the original patch with the new function releaseDelayedReleasedObjects().
+
+        Updated releaseDelayedReleasedObjects() so that objects are released with all locks
+        dropped.  Now its processing follows these steps
+            Increment recursion counter and do recursion check and exit if recursing
+            While there are objects to release
+                ASSERT that lock is held by current thread
+                Take all items from delayed release Vector and put into temporary Vector
+                Release API lock
+                Release and clear items from temporary vector
+                Reaquire API lock
+        This meets the requirement that we release while the API lock is released and it is
+        safer processing of the delayed release Vector.
+
+        Added new regression test to testapi.
+
+        Also added comment describing how recursion into releaseDelayedReleasedObjects() is
+        prevented.
+
+        * API/tests/Regress141275.h: Added.
+        * API/tests/Regress141275.mm: Added.
+        (+[JSTEvaluatorTask evaluatorTaskWithEvaluateBlock:completionHandler:]):
+        (-[JSTEvaluator init]):
+        (-[JSTEvaluator initWithScript:]):
+        (-[JSTEvaluator _accessPendingTasksWithBlock:]):
+        (-[JSTEvaluator insertSignPostWithCompletion:]):
+        (-[JSTEvaluator evaluateScript:completion:]):
+        (-[JSTEvaluator evaluateBlock:completion:]):
+        (-[JSTEvaluator waitForTasksDoneAndReportResults]):
+        (__JSTRunLoopSourceScheduleCallBack):
+        (__JSTRunLoopSourcePerformCallBack):
+        (__JSTRunLoopSourceCancelCallBack):
+        (-[JSTEvaluator _jsThreadMain]):
+        (-[JSTEvaluator _sourceScheduledOnRunLoop:]):
+        (-[JSTEvaluator _setupEvaluatorThreadContextIfNeeded]):
+        (-[JSTEvaluator _callCompletionHandler:ifNeededWithError:]):
+        (-[JSTEvaluator _sourcePerform]):
+        (-[JSTEvaluator _sourceCanceledOnRunLoop:]):
+        (runRegress141275):
+        * API/tests/testapi.mm:
+        (testObjectiveCAPI):
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        * heap/Heap.cpp:
+        (JSC::Heap::releaseDelayedReleasedObjects):
+        * runtime/JSLock.cpp:
+        (JSC::JSLock::unlock):
+
 2015-03-03  Filip Pizlo  <fpizlo@apple.com>
 
         DFG should constant fold GetScope, and accesses to the scope register in the ByteCodeParser should not pretend that it's a constant as that breaks OSR exit liveness tracking
index 6a845658a274b2e33e9e9d7487f1e1931da02140..cf5f90ed3f6b09b5e74b680bc48982c167d7a636 100644 (file)
                65525FC51A6DD801007B5495 /* NullSetterFunction.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 65525FC31A6DD3B3007B5495 /* NullSetterFunction.cpp */; };
                6553A33117A1F1EE008CF6F3 /* CommonSlowPathsExceptions.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6553A32F17A1F1EE008CF6F3 /* CommonSlowPathsExceptions.cpp */; };
                6553A33217A1F1EE008CF6F3 /* CommonSlowPathsExceptions.h in Headers */ = {isa = PBXBuildFile; fileRef = 6553A33017A1F1EE008CF6F3 /* CommonSlowPathsExceptions.h */; };
+               65570F5A1AA4C3EA009B3C23 /* Regress141275.mm in Sources */ = {isa = PBXBuildFile; fileRef = 65570F591AA4C00A009B3C23 /* Regress141275.mm */; };
                655EB29B10CE2581001A990E /* NodesCodegen.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 655EB29A10CE2581001A990E /* NodesCodegen.cpp */; };
                657CF45819BF6662004ACBF2 /* JSCallee.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 657CF45619BF6662004ACBF2 /* JSCallee.cpp */; };
                657CF45919BF6662004ACBF2 /* JSCallee.h in Headers */ = {isa = PBXBuildFile; fileRef = 657CF45719BF6662004ACBF2 /* JSCallee.h */; settings = {ATTRIBUTES = (Private, ); }; };
                65525FC41A6DD3B3007B5495 /* NullSetterFunction.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = NullSetterFunction.h; sourceTree = "<group>"; };
                6553A32F17A1F1EE008CF6F3 /* CommonSlowPathsExceptions.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CommonSlowPathsExceptions.cpp; sourceTree = "<group>"; };
                6553A33017A1F1EE008CF6F3 /* CommonSlowPathsExceptions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CommonSlowPathsExceptions.h; sourceTree = "<group>"; };
+               65570F581AA4C00A009B3C23 /* Regress141275.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = Regress141275.h; path = API/tests/Regress141275.h; sourceTree = "<group>"; };
+               65570F591AA4C00A009B3C23 /* Regress141275.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = Regress141275.mm; path = API/tests/Regress141275.mm; sourceTree = "<group>"; };
                655EB29A10CE2581001A990E /* NodesCodegen.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = NodesCodegen.cpp; sourceTree = "<group>"; };
                6560A4CF04B3B3E7008AE952 /* CoreFoundation.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = CoreFoundation.framework; path = /System/Library/Frameworks/CoreFoundation.framework; sourceTree = "<absolute>"; };
                65621E6B089E859700760F35 /* PropertySlot.cpp */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PropertySlot.cpp; sourceTree = "<group>"; tabWidth = 8; };
                                C288B2DD18A54D3E007BE40B /* DateTests.mm */,
                                C2181FC018A948FB0025A235 /* JSExportTests.h */,
                                C2181FC118A948FB0025A235 /* JSExportTests.mm */,
+                               65570F581AA4C00A009B3C23 /* Regress141275.h */,
+                               65570F591AA4C00A009B3C23 /* Regress141275.mm */,
                                FEB51F6A1A97B688001F921C /* Regress141809.h */,
                                FEB51F6B1A97B688001F921C /* Regress141809.mm */,
                                144005170A531CB50005F061 /* minidom */,
                        isa = PBXSourcesBuildPhase;
                        buildActionMask = 2147483647;
                        files = (
+                               65570F5A1AA4C3EA009B3C23 /* Regress141275.mm in Sources */,
                                C29ECB031804D0ED00D2CBB4 /* CurrentThisInsideBlockGetterTest.mm in Sources */,
                                FEB51F6C1A97B688001F921C /* Regress141809.mm in Sources */,
                                C20328201981979D0088B499 /* CustomGlobalObjectClassTest.c in Sources */,
index 4fcce7020aaca8cfa626718ee3d1910e20fd53ce..41acec21d5c85011becadd585b2b491a8975e6cc 100644 (file)
@@ -368,10 +368,25 @@ void Heap::lastChanceToFinalize()
 void Heap::releaseDelayedReleasedObjects()
 {
 #if USE(CF)
+    // We need to guard against the case that releasing an object can create more objects due to the
+    // release calling into JS. When those JS call(s) exit and all locks are being dropped we end up
+    // back here and could try to recursively release objects. We guard that with a recursive entry
+    // count. Only the initial call will release objects, recursive calls simple return and let the
+    // the initial call to the function take care of any objects created during release time.
+    // This also means that we need to loop until there are no objects in m_delayedReleaseObjects
+    // and use a temp Vector for the actual releasing.
     if (!m_delayedReleaseRecursionCount++) {
         while (!m_delayedReleaseObjects.isEmpty()) {
-            RetainPtr<CFTypeRef> objectToRelease = m_delayedReleaseObjects.takeLast();
-            objectToRelease.clear();
+            ASSERT(m_vm->currentThreadIsHoldingAPILock());
+
+            Vector<RetainPtr<CFTypeRef>> objectsToRelease = WTF::move(m_delayedReleaseObjects);
+
+            {
+                // We need to drop locks before calling out to arbitrary code.
+                JSLock::DropAllLocks dropAllLocks(m_vm);
+
+                objectsToRelease.clear();
+            }
         }
     }
     m_delayedReleaseRecursionCount--;
index 568e746c0be3a2f7eaf3ab59a02c2e95f530236b..9defebfcf51a391231f849de26044938a41f5d25 100644 (file)
@@ -157,10 +157,14 @@ void JSLock::unlock(intptr_t unlockCount)
     RELEASE_ASSERT(currentThreadIsHoldingLock());
     ASSERT(m_lockCount >= unlockCount);
 
+    // Maintain m_lockCount while calling willReleaseLock() so that its callees know that
+    // they still have the lock.
+    if (unlockCount == m_lockCount)
+        willReleaseLock();
+
     m_lockCount -= unlockCount;
 
     if (!m_lockCount) {
-        willReleaseLock();
 
         if (!m_hasExclusiveThread) {
             m_ownerThreadID = std::thread::id();