better kill testing

rbri · rbri · commit 7a2873b3b112 · 2025-01-10T16:16:30.000+01:00
diff --git a/src/main/java/org/htmlunit/javascript/background/DefaultJavaScriptExecutor.java b/src/main/java/org/htmlunit/javascript/background/DefaultJavaScriptExecutor.java
@@ -102,7 +102,13 @@ private void killThread() {
             }
 
             // Stop the thread
-            eventLoopThread_.stop();
+            try {
+                eventLoopThread_.stop();
+            }
+            catch (final Exception e) {
+                LOG.warn("JS thread did not interrupt after 10s, maybe there is an endless loop."
+                        + "Please consider setting an JavaScriptTimeout for the WebClient.", e);
+            }
         }
     }
 
@@ -174,6 +180,8 @@ public void run() {
             catch (final InterruptedException e) {
                 // restore interrupted status
                 Thread.currentThread().interrupt();
+
+                break;
             }
         }
     }
diff --git a/src/test/java/org/htmlunit/javascript/JavaScriptEngineTest.java b/src/test/java/org/htmlunit/javascript/JavaScriptEngineTest.java
@@ -1182,6 +1182,44 @@ public void shutdownShouldKill() throws Exception {
                 + "</body></html>";
 
         try (WebClient webClient = getWebClient()) {
+            // there is no way to kill a thread in later JDK's
+            // to make the test running we need a final timeout for js stuff
+            webClient.setJavaScriptTimeout(DEFAULT_WAIT_TIME * 20);
+
+            final List<String> collectedAlerts = new ArrayList<>();
+            webClient.setAlertHandler(new CollectingAlertHandler(collectedAlerts));
+
+            loadPage(html);
+            Thread.sleep(100);
+            assertEquals(getExpectedAlerts(), collectedAlerts);
+
+        }
+        Thread.sleep(400);
+        assertTrue(getJavaScriptThreads().isEmpty());
+    }
+
+    /**
+     * @throws Exception if the test fails
+     */
+    @Test
+    @Retry
+    @Alerts("starting")
+    public void shutdownShouldKillJavaScriptTimeout() throws Exception {
+        final String html = "<html>\n"
+                + "<head><title>Test page</title>\n"
+                + "<script>\n"
+                + "  function test() {\n"
+                + "    alert('starting');\n"
+                + "    while(true) { Math.sin(3.14) * Math.sin(3.14);}\n"
+                + "  }\n"
+                + "</script>\n"
+                + "</head>\n"
+                + "<body onload='setTimeout(test, 50);'>\n"
+                + "</body></html>";
+
+        try (WebClient webClient = getWebClient()) {
+            webClient.setJavaScriptTimeout(DEFAULT_WAIT_TIME);
+
             final List<String> collectedAlerts = new ArrayList<>();
             webClient.setAlertHandler(new CollectingAlertHandler(collectedAlerts));
 

Original file line number	Diff line number	Diff line change
`@@ -102,7 +102,13 @@ private void killThread() {`
`102`	`102`	`}`
`103`	`103`
`104`	`104`	`// Stop the thread`
`105`		`- eventLoopThread_.stop();`
	`105`	`+ try {`
	`106`	`+ eventLoopThread_.stop();`
	`107`	`+ }`
	`108`	`+ catch (final Exception e) {`
	`109`	`+ LOG.warn("JS thread did not interrupt after 10s, maybe there is an endless loop."`
	`110`	`+ + "Please consider setting an JavaScriptTimeout for the WebClient.", e);`
	`111`	`+ }`
`106`	`112`	`}`
`107`	`113`	`}`
`108`	`114`
`@@ -174,6 +180,8 @@ public void run() {`
`174`	`180`	`catch (final InterruptedException e) {`
`175`	`181`	`// restore interrupted status`
`176`	`182`	`Thread.currentThread().interrupt();`
	`183`	`+`
	`184`	`+ break;`
`177`	`185`	`}`
`178`	`186`	`}`
`179`	`187`	`}`