Changeset 182661 in webkit

Timestamp:

Apr 11, 2015 12:43:34 PM (9 years ago)

Author:

Yusuke Suzuki

Message:

Run flaky conservative GC related test first before polluting stack and registers
​https://bugs.webkit.org/show_bug.cgi?id=143634

Reviewed by Ryosuke Niwa.

After r182653, JSC API tests fail. However, it's not related to the change.
After investigating the cause of this failure, I've found that the failed test is flaky
because JSC's GC is conservative. If previously allocated JSGlobalObject is accidentally alive
due to conservative roots in C stack and registers, this test fails.

Since GC marks C stack and registers as roots conservatively,
objects not referenced logically can be accidentally marked and alive.
To avoid this situation as possible as we can,

1. run this test first before stack is polluted,
2. extract this test as a function to suppress stack height.

• API/tests/testapi.mm:

(testWeakValue):
(testObjectiveCAPIMain):
(testObjectiveCAPI):

Location:

trunk/Source/JavaScriptCore

Files:

• API/tests/testapi.mm (modified) (3 diffs)
• ChangeLog (modified) (1 diff)

trunk/Source/JavaScriptCore/API/tests/testapi.mm

@@ -485 +485 @@
 }
 
-void testObjectiveCAPI()
-{
-    NSLog(@"Testing Objective-C API");
+// This test is flaky. Since GC marks C stack and registers as roots conservatively,
+// objects not referenced logically can be accidentally marked and alive.
+// To avoid this situation as possible as we can,
+// 1. run this test first before stack is polluted,
+// 2. extract this test as a function to suppress stack height.
+static void testWeakValue()
+{
+    @autoreleasepool {
+        JSVirtualMachine *vm = [[JSVirtualMachine alloc] init];
+        TestObject *testObject = [TestObject testObject];
+        JSManagedValue *weakValue;
+        @autoreleasepool {
+            JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
+            context[@"testObject"] = testObject;
+            weakValue = [[JSManagedValue alloc] initWithValue:context[@"testObject"]];
+        }
+
+        @autoreleasepool {
+            JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
+            context[@"testObject"] = testObject;
+            JSSynchronousGarbageCollectForDebugging([context JSGlobalContextRef]);
+            checkResult(@"weak value == nil", ![weakValue value]);
+            checkResult(@"root is still alive", !context[@"testObject"].isUndefined);
+        }
+    }
+}
+
+static void testObjectiveCAPIMain()
+{
     @autoreleasepool {
         JSVirtualMachine* vm = [[JSVirtualMachine alloc] init];
@@ -1060 +1086 @@
             JSValue *result = [context evaluateScript:@"didClick"];
             checkResult(@"Event handler onclick doesn't fire", ![result toBool]);
-        }
-    }
-
-    @autoreleasepool {
-        JSVirtualMachine *vm = [[JSVirtualMachine alloc] init];
-        TestObject *testObject = [TestObject testObject];
-        JSManagedValue *weakValue;
-        @autoreleasepool {
-            JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
-            context[@"testObject"] = testObject;
-            weakValue = [[JSManagedValue alloc] initWithValue:context[@"testObject"]];
-        }
-
-        @autoreleasepool {
-            JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
-            context[@"testObject"] = testObject;
-            JSSynchronousGarbageCollectForDebugging([context JSGlobalContextRef]);
-            checkResult(@"weak value == nil", ![weakValue value]);
-            checkResult(@"root is still alive", !context[@"testObject"].isUndefined);
         }
     }
@@ -1410 +1417 @@
 }
 
+void testObjectiveCAPI()
+{
+    NSLog(@"Testing Objective-C API");
+    testWeakValue();
+    testObjectiveCAPIMain();
+}
+
 #else
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2015-04-11  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        Run flaky conservative GC related test first before polluting stack and registers
+        https://bugs.webkit.org/show_bug.cgi?id=143634
+
+        Reviewed by Ryosuke Niwa.
+
+        After r182653, JSC API tests fail. However, it's not related to the change.
+        After investigating the cause of this failure, I've found that the failed test is flaky
+        because JSC's GC is conservative. If previously allocated JSGlobalObject is accidentally alive
+        due to conservative roots in C stack and registers, this test fails.
+
+        Since GC marks C stack and registers as roots conservatively,
+        objects not referenced logically can be accidentally marked and alive.
+        To avoid this situation as possible as we can,
+        1. run this test first before stack is polluted,
+        2. extract this test as a function to suppress stack height.
+
+        * API/tests/testapi.mm:
+        (testWeakValue):
+        (testObjectiveCAPIMain):
+        (testObjectiveCAPI):
+
 2015-04-11  Matt Baker  <mattbaker@apple.com>