[JSC] Do not run testPingPongStackOverflow while running multithreaded MultithreadedMultiVMExecutionTest
https://bugs.webkit.org/show_bug.cgi?id=235633

Reviewed by Mark Lam.

MultithreadedMultiVMExecutionTest is failing occasionally in CLoop test. This is because of the following.

1. CLoop is slow, so multithreaded tests are running longly.
2. Then, this multithreaded tests overlap with testPingPongStackOverflow.
3. testPingPongStackOverflow changes global Options::maxPerThreadStackUsage to test stack-overflow behavior.
   This test is strongly assuming that there is only one thread using this VM. But this is wrong since
   MultithreadedMultiVMExecutionTest is running concurrently. Then this configuration change affects on
   the running MultithreadedMultiVMExecutionTest.
4. Stack-overflow error happens in MultithreadedMultiVMExecutionTest if the changed option is observed in that test.

We should not run testPingPongStackOverflow until MultithreadedMultiVMExecutionTest finishes since it assumes
that there is only one user of this VM.

This patch also cleans up / adds diagnosis of failures in MultithreadedMultiVMExecutionTest.

* API/tests/MultithreadedMultiVMExecutionTest.cpp:
(startMultithreadedMultiVMExecutionTest):
(finalizeMultithreadedMultiVMExecutionTest):
* API/tests/testapi.c:
(main):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@288612 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/API/tests/MultithreadedMultiVMExecutionTest.cpp b/Source/JavaScriptCore/API/tests/MultithreadedMultiVMExecutionTest.cpp
index 5862337..5f26637 100644
--- a/Source/JavaScriptCore/API/tests/MultithreadedMultiVMExecutionTest.cpp
+++ b/Source/JavaScriptCore/API/tests/MultithreadedMultiVMExecutionTest.cpp
@@ -51,14 +51,14 @@
     WTF::initializeMainThread();
     JSC::initialize();
 
-#define CHECK(condition, message) do { \
+#define CHECK(condition, threadNumber, count, message) do { \
         if (!condition) { \
-            printf("FAILED MultithreadedMultiVMExecutionTest: %s\n", message); \
+            printf("FAIL: MultithreadedMultiVMExecutionTest: %d %d %s\n", threadNumber, count, message); \
             failuresFound++; \
         } \
     } while (false)
 
-    auto task = [&]() {
+    auto task = [](int threadNumber) {
         int ret = 0;
         std::string scriptString =
             "const AAA = {A:0, B:1, C:2, D:3};"
@@ -67,17 +67,30 @@
 
         for (int i = 0; i < 1000; ++i) {
             JSClassRef jsClass = JSClassCreate(&kJSClassDefinitionEmpty);
-            CHECK(jsClass, "global object class creation");
+            CHECK(jsClass, threadNumber, i, "global object class creation");
             JSContextGroupRef contextGroup = JSContextGroupCreate();
-            CHECK(contextGroup, "group creation");
+            CHECK(contextGroup, threadNumber, i, "group creation");
             JSGlobalContextRef context = JSGlobalContextCreateInGroup(contextGroup, jsClass);
-            CHECK(context, "ctx creation");
+            CHECK(context, threadNumber, i, "ctx creation");
 
             JSStringRef jsScriptString = JSStringCreateWithUTF8CString(scriptString.c_str());
-            CHECK(jsScriptString, "script to jsString");
+            CHECK(jsScriptString, threadNumber, i, "script to jsString");
 
-            JSValueRef jsScript = JSEvaluateScript(context, jsScriptString, nullptr, nullptr, 0, nullptr);
-            CHECK(jsScript, "script eval");
+            JSValueRef exception = nullptr;
+            JSValueRef jsScript = JSEvaluateScript(context, jsScriptString, nullptr, nullptr, 0, &exception);
+            CHECK(!exception, threadNumber, i, "script eval no exception");
+            if (exception) {
+                JSStringRef string = JSValueToStringCopy(context, exception, nullptr);
+                if (string) {
+                    std::vector<char> buffer;
+                    buffer.resize(JSStringGetMaximumUTF8CStringSize(string));
+                    JSStringGetUTF8CString(string, buffer.data(), buffer.size());
+                    printf("FAIL: MultithreadedMultiVMExecutionTest: %d %d %s\n", threadNumber, i, buffer.data());
+                    JSStringRelease(string);
+                } else
+                    printf("FAIL: MultithreadedMultiVMExecutionTest: %d %d stringifying exception failed\n", threadNumber, i);
+            }
+            CHECK(jsScript, threadNumber, i, "script eval");
             JSStringRelease(jsScriptString);
 
             JSGlobalContextRelease(context);
@@ -88,7 +101,7 @@
         return ret;
     };
     for (int t = 0; t < 8; ++t)
-        threadsList().push_back(std::thread(task));
+        threadsList().push_back(std::thread(task, t));
 }
 
 int finalizeMultithreadedMultiVMExecutionTest()
@@ -97,7 +110,6 @@
     for (auto& thread : threads)
         thread.join();
 
-    if (failuresFound)
-        printf("FAILED MultithreadedMultiVMExecutionTest\n");
+    printf("%s: MultithreadedMultiVMExecutionTest\n", failuresFound ? "FAIL" : "PASS");
     return (failuresFound > 0);
 }
diff --git a/Source/JavaScriptCore/API/tests/testapi.c b/Source/JavaScriptCore/API/tests/testapi.c
index 4c4760a..b6aa356 100644
--- a/Source/JavaScriptCore/API/tests/testapi.c
+++ b/Source/JavaScriptCore/API/tests/testapi.c
@@ -1622,7 +1622,7 @@
         failed = 1;
     } else
         printf("PASS: Correctly returned null for invalid JSON data.\n");
-    JSValueRef exception;
+    JSValueRef exception = NULL;
     JSStringRef str = JSValueCreateJSONString(context, jsonObject, 0, 0);
     if (!JSStringIsEqualToUTF8CString(str, "{\"aProperty\":true}")) {
         printf("FAIL: Did not correctly serialise with indent of 0.\n");
@@ -2119,7 +2119,6 @@
     failed |= testTypedArrayCAPI();
     failed |= testFunctionOverrides();
     failed |= testGlobalContextWithFinalizer();
-    failed |= testPingPongStackOverflow();
     failed |= testJSONParse();
     failed |= testJSObjectGetProxyTarget();
 
@@ -2170,14 +2169,18 @@
     globalObjectSetPrototypeTest();
     globalObjectPrivatePropertyTest();
 
-    failed = finalizeMultithreadedMultiVMExecutionTest() || failed;
+    failed |= finalizeMultithreadedMultiVMExecutionTest();
 
-    // Don't run this till after the MultithreadedMultiVMExecutionTest has finished.
-    // This is because testExecutionTimeLimit() modifies JIT options at runtime
+    // Don't run these tests till after the MultithreadedMultiVMExecutionTest has finished.
+    // 1. testPingPongStackOverflow() changes stack size per thread configuration at runtime to very small value,
+    // which can cause stack-overflow on MultithreadedMultiVMExecutionTest test.
+    // 2. testExecutionTimeLimit() modifies JIT options at runtime
     // as part of its testing. This can wreak havoc on the rest of the system that
     // expects the options to be frozen. Ideally, we'll find a way for testExecutionTimeLimit()
     // to do its work without changing JIT options, but that is not easy to do.
-    // For now, we'll just run it here at the end as a workaround.
+    //
+    // For now, we'll just run them here at the end as a workaround.
+    failed |= testPingPongStackOverflow();
     failed |= testExecutionTimeLimit();
 
     if (failed) {
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 10fad42..f8d318b 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,31 @@
+2022-01-26  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Do not run testPingPongStackOverflow while running multithreaded MultithreadedMultiVMExecutionTest
+        https://bugs.webkit.org/show_bug.cgi?id=235633
+
+        Reviewed by Mark Lam.
+
+        MultithreadedMultiVMExecutionTest is failing occasionally in CLoop test. This is because of the following.
+
+        1. CLoop is slow, so multithreaded tests are running longly.
+        2. Then, this multithreaded tests overlap with testPingPongStackOverflow.
+        3. testPingPongStackOverflow changes global Options::maxPerThreadStackUsage to test stack-overflow behavior.
+           This test is strongly assuming that there is only one thread using this VM. But this is wrong since
+           MultithreadedMultiVMExecutionTest is running concurrently. Then this configuration change affects on
+           the running MultithreadedMultiVMExecutionTest.
+        4. Stack-overflow error happens in MultithreadedMultiVMExecutionTest if the changed option is observed in that test.
+
+        We should not run testPingPongStackOverflow until MultithreadedMultiVMExecutionTest finishes since it assumes
+        that there is only one user of this VM.
+
+        This patch also cleans up / adds diagnosis of failures in MultithreadedMultiVMExecutionTest.
+
+        * API/tests/MultithreadedMultiVMExecutionTest.cpp:
+        (startMultithreadedMultiVMExecutionTest):
+        (finalizeMultithreadedMultiVMExecutionTest):
+        * API/tests/testapi.c:
+        (main):
+
 2022-01-25  Mark Lam  <mark.lam@apple.com>
 
         Gardening: build fix for CLoop.