Fixed tests and race condition potential

Signed-off-by: Viet Nguyen Duc <[email protected]>
SeleniumHQ · Oct 22, 2024 · a7a838a · a7a838a
1 parent b0f3cd9
commit a7a838a
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 84 deletions.
diff --git a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java
@@ -944,18 +944,20 @@ public void drain() {
   }
 
   private void checkSessionCount() {
-    if (this.drainAfterSessions.get()) {
-      int remainingSessions = this.sessionCount.decrementAndGet();
-      LOG.log(
-          Debug.getDebugLogLevel(),
-          "{0} remaining sessions before draining Node",
-          remainingSessions);
-      if (remainingSessions <= 0) {
-        LOG.info(
-            String.format(
-                "Draining Node, configured sessions value (%s) has been reached.",
-                this.configuredSessionCount));
-        drain();
+    synchronized (sessionCount) {
+      if (this.drainAfterSessions.get()) {
+        int remainingSessions = this.sessionCount.decrementAndGet();
+        LOG.log(
+            Debug.getDebugLogLevel(),
+            "{0} remaining sessions before draining Node",
+            remainingSessions);
+        if (remainingSessions <= 0) {
+          LOG.info(
+              String.format(
+                  "Draining Node, configured sessions value (%s) has been reached.",
+                  this.configuredSessionCount));
+          drain();
+        }
       }
     }
   }

diff --git a/java/test/org/openqa/selenium/grid/e2e/BUILD.bazel b/java/test/org/openqa/selenium/grid/e2e/BUILD.bazel
diff --git a/java/test/org/openqa/selenium/grid/e2e/SessionTimeoutTest.java b/java/test/org/openqa/selenium/grid/e2e/SessionTimeoutTest.java
diff --git a/java/test/org/openqa/selenium/grid/router/StressTest.java b/java/test/org/openqa/selenium/grid/router/StressTest.java
@@ -20,6 +20,8 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.concurrent.TimeUnit.MINUTES;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.StringReader;
 import java.util.LinkedList;
@@ -33,6 +35,7 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.openqa.selenium.By;
+import org.openqa.selenium.NoSuchSessionException;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.grid.config.MapConfig;
 import org.openqa.selenium.grid.config.MemoizedConfig;
@@ -65,7 +68,12 @@ public void setupServers() {
         DeploymentTypes.DISTRIBUTED.start(
             browser.getCapabilities(),
             new TomlConfig(
-                new StringReader("[node]\n" + "driver-implementation = " + browser.displayName())));
+                new StringReader(
+                    "[node]\n"
+                        + "driver-implementation = "
+                        + browser.displayName()
+                        + "\n"
+                        + "session-timeout = 15")));
     tearDowns.add(deployment);
 
     server = deployment.getServer();
@@ -124,4 +132,15 @@ void multipleSimultaneousSessions() throws Exception {
 
     CompletableFuture.allOf(futures).get(4, MINUTES);
   }
+
+  @Test
+  void testStopTimedOutSession() throws Exception {
+    assertThat(server.isStarted()).isTrue();
+    WebDriver driver =
+        RemoteWebDriver.builder().oneOf(browser.getCapabilities()).address(server.getUrl()).build();
+    driver.get(appServer.getUrl().toString());
+    Thread.sleep(15000);
+    NoSuchSessionException exception = assertThrows(NoSuchSessionException.class, driver::getTitle);
+    assertTrue(exception.getMessage().startsWith("Cannot find session with id:"));
+  }
 }