Skip to content

Commit 9dced98

Browse files
committed
[grid] Update for node shutdown gracefully
Signed-off-by: Viet Nguyen Duc <[email protected]>
1 parent 6ea5651 commit 9dced98

File tree

4 files changed

+86
-5
lines changed

4 files changed

+86
-5
lines changed

java/src/org/openqa/selenium/grid/node/Node.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,10 @@ protected Node(
152152
req ->
153153
getSessionId(req.getUri())
154154
.map(SessionId::new)
155-
.map(this::isSessionOwner)
155+
.map(
156+
sessionId ->
157+
this.getSession(sessionId) != null
158+
&& this.isSessionOwner(sessionId))
156159
.orElse(false))
157160
.to(() -> new ForwardWebDriverCommand(this))
158161
.with(spanDecorator("node.forward_command")),

java/src/org/openqa/selenium/grid/node/local/LocalNode.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,13 @@ protected LocalNode(
297297
heartbeatPeriod.getSeconds(),
298298
TimeUnit.SECONDS);
299299

300-
Runtime.getRuntime().addShutdownHook(new Thread(this::stopAllSessions));
300+
Runtime.getRuntime()
301+
.addShutdownHook(
302+
new Thread(
303+
() -> {
304+
stopAllSessions();
305+
drain();
306+
}));
301307
new JMXHelper().register(this);
302308
}
303309

@@ -316,7 +322,6 @@ private void stopTimedOutSession(RemovalNotification<SessionId, SessionSlot> not
316322
}
317323
// Attempt to stop the session
318324
slot.stop();
319-
this.sessionToDownloadsDir.invalidate(id);
320325
// Decrement pending sessions if Node is draining
321326
if (this.isDraining()) {
322327
int done = pendingSessions.decrementAndGet();
@@ -473,8 +478,6 @@ public Either<WebDriverException, CreateSessionResponse> newSession(
473478
sessionToDownloadsDir.put(session.getId(), uuidForSessionDownloads);
474479
currentSessions.put(session.getId(), slotToUse);
475480

476-
checkSessionCount();
477-
478481
SessionId sessionId = session.getId();
479482
Capabilities caps = session.getCapabilities();
480483
SESSION_ID.accept(span, sessionId);
@@ -513,6 +516,8 @@ public Either<WebDriverException, CreateSessionResponse> newSession(
513516
span.addEvent("Unable to create session with the driver", attributeMap);
514517
return Either.left(possibleSession.left());
515518
}
519+
} finally {
520+
checkSessionCount();
516521
}
517522
}
518523

@@ -765,6 +770,10 @@ public HttpResponse uploadFile(HttpRequest req, SessionId id) {
765770
public void stop(SessionId id) throws NoSuchSessionException {
766771
Require.nonNull("Session ID", id);
767772

773+
if (sessionToDownloadsDir.getIfPresent(id) != null) {
774+
sessionToDownloadsDir.invalidate(id);
775+
}
776+
768777
SessionSlot slot = currentSessions.getIfPresent(id);
769778
if (slot == null) {
770779
throw new NoSuchSessionException("Cannot find session with id: " + id);
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
load("@rules_jvm_external//:defs.bzl", "artifact")
2+
load("//java:defs.bzl", "JUNIT5_DEPS", "java_selenium_test_suite")
3+
4+
java_selenium_test_suite(
5+
name = "large-tests",
6+
size = "large",
7+
srcs = glob(["*.java"]),
8+
browsers = [
9+
"chrome",
10+
],
11+
deps = [
12+
"//java/src/org/openqa/selenium:core",
13+
"//java/src/org/openqa/selenium/chrome",
14+
"//java/src/org/openqa/selenium/grid",
15+
"//java/src/org/openqa/selenium/remote/http",
16+
artifact("com.google.guava:guava"),
17+
artifact("org.junit.jupiter:junit-jupiter-api"),
18+
artifact("org.assertj:assertj-core"),
19+
] + JUNIT5_DEPS,
20+
)
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.openqa.selenium.grid.e2e;
19+
20+
import static org.junit.jupiter.api.Assertions.assertThrows;
21+
import static org.junit.jupiter.api.Assertions.assertTrue;
22+
23+
import java.net.URI;
24+
import org.junit.jupiter.api.Test;
25+
import org.openqa.selenium.NoSuchSessionException;
26+
import org.openqa.selenium.WebDriver;
27+
import org.openqa.selenium.chrome.ChromeOptions;
28+
import org.openqa.selenium.grid.Bootstrap;
29+
import org.openqa.selenium.remote.RemoteWebDriver;
30+
31+
public class SessionTimeoutTest {
32+
@Test
33+
void testSessionTimeout() throws Exception {
34+
Bootstrap.main(("hub --host 127.0.0.1 --port 4444").split(" "));
35+
Bootstrap.main(
36+
("node --host 127.0.0.1 --port 5555 --session-timeout 15 --selenium-manager true")
37+
.split(" "));
38+
39+
var options = new ChromeOptions();
40+
options.addArguments("--disable-search-engine-choice-screen");
41+
options.addArguments("--headless=new");
42+
43+
WebDriver driver = new RemoteWebDriver(URI.create("http://localhost:4444").toURL(), options);
44+
driver.get("http://localhost:4444");
45+
Thread.sleep(15000);
46+
NoSuchSessionException exception = assertThrows(NoSuchSessionException.class, driver::getTitle);
47+
assertTrue(exception.getMessage().startsWith("Cannot find session with id:"));
48+
}
49+
}

0 commit comments

Comments
 (0)