Restore the interrupt status in UrlChecker.waitUntilUnavailable by vasiliy-mikhailov · Pull Request #17719 · SeleniumHQ/selenium

vasiliy-mikhailov · 2026-06-26T06:04:49Z

UrlChecker.waitUntilUnavailable catches InterruptedException and rethrows it wrapped in a RuntimeException without re-setting the thread's interrupt flag, so a caller can no longer tell the wait was interrupted. The sibling waitUntilAvailable already restores the flag in the same place, so this just makes the two consistent.

The added test interrupts the calling thread while waitUntilUnavailable is waiting and asserts the interrupt status is preserved.

AI assistance disclosure

This contribution was produced with the help of an AI pipeline. The pipeline processed a large amount of source code to surface suspected bugs, reproduced a subset of them with failing unit tests and generated candidate fixes, and prepared pull requests from the ones that held up. Each PR was then reviewed and verified by a human before being opened: the fix and test were checked by hand and the test was confirmed to fail before the change and pass after.

waitUntilUnavailable caught InterruptedException and rethrew it wrapped in a RuntimeException without re-setting the thread interrupt flag, unlike the sibling waitUntilAvailable. Re-set the flag so callers can still observe the interruption.

qodo-code-review · 2026-06-26T06:05:43Z

PR Summary by Qodo

Restore interrupt status in UrlChecker.waitUntilUnavailable
🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Description

• Preserve thread interrupt status when waitUntilUnavailable is interrupted.
• Align interruption handling with existing waitUntilAvailable behavior.
• Add regression test asserting interrupt flag remains set after interruption.

Diagram

graph TD
A["Caller thread (test)"] --> B["UrlChecker.waitUntilUnavailable"] --> C["Wait on Future.get"] --> D{"Interrupted?"}
D -->|"yes"| E["Re-interrupt current thread"] --> F["Throw RuntimeException"]
D -->|"no"| G["Handle ExecutionException/timeout"] --> F

High-Level Assessment

The following are alternative approaches to this PR:

1. Propagate InterruptedException (checked) instead of wrapping

➕ Makes interruption explicit to callers via the type system
➕ Avoids RuntimeException-based control flow
➖ Requires API signature change and downstream call-site updates
➖ Inconsistent with existing sibling method behavior if it keeps wrapping

2. Wrap in a dedicated UncheckedInterruptedException

➕ Preserves interrupt semantics while communicating intent more clearly than RuntimeException
➕ Can be handled distinctly from other runtime failures
➖ Introduces a new exception type/API surface
➖ Still requires caller awareness and documentation

Recommendation: The chosen approach (re-interrupt then wrap) is appropriate because it preserves interruption semantics without changing the public API, and it matches waitUntilAvailable’s established behavior. The main alternative—throwing InterruptedException—would be cleaner but is a breaking change.

Files changed (2) +33 / -1

Bug fix (1) +4 / -1

UrlChecker.javaRe-interrupt thread when waitUntilUnavailable is interrupted +4/-1

Re-interrupt thread when waitUntilUnavailable is interrupted
• Splits the multi-catch so InterruptedException is handled separately. Restores the current thread's interrupt flag before rethrowing as a RuntimeException, keeping behavior consistent with waitUntilAvailable.
java/src/org/openqa/selenium/net/UrlChecker.java

Tests (1) +29 / -0

UrlCheckerTest.javaAdd regression test for interrupt preservation in waitUntilUnavailable +29/-0

Add regression test for interrupt preservation in waitUntilUnavailable
• Adds a test that interrupts the calling thread while waitUntilUnavailable is blocked. Asserts a RuntimeException is thrown and the thread interrupt status remains set (then clears it to avoid leaking state).
java/test/org/openqa/selenium/net/UrlCheckerTest.java

qodo-code-review · 2026-06-26T06:06:45Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 18 rules

1. ~~Interrupt test doesn’t block~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

waitUntilUnavailablePreservesInterruptStatus calls waitUntilUnavailable while the
NettyAppServer created in @BeforeEach is never started, so waitUntilUnavailable can return
immediately on IOException and never throw the expected RuntimeException. As written, the test
can fail (no exception thrown) and does not reliably validate that the interrupt status is preserved
during a blocking wait.

Code

java/test/org/openqa/selenium/net/UrlCheckerTest.java[R106-133]

+  @Test
+  void waitUntilUnavailablePreservesInterruptStatus() throws Exception {
+    Thread caller = Thread.currentThread();
+    Thread interrupter =
+        new Thread(
+            () -> {
+              try {
+                Thread.sleep(500);
+              } catch (InterruptedException ignored) {
+              }
+              caller.interrupt();
+            });
+    interrupter.start();
+
+    boolean threw = false;
+    boolean preserved = false;
+    try {
+      urlChecker.waitUntilUnavailable(10, TimeUnit.SECONDS, url);
+    } catch (RuntimeException expected) {
+      threw = true;
+      preserved = Thread.currentThread().isInterrupted();
+      Thread.interrupted();
+    }
+    interrupter.join();
+
+    assertThat(threw).isTrue();
+    assertThat(preserved).isTrue();
+  }

Evidence
The test’s server is never started, and waitUntilUnavailable returns null immediately on
IOException, so the test may never reach the interruption/exception path it asserts on.
java/test/org/openqa/selenium/net/UrlCheckerTest.java[42-57]
java/test/org/openqa/selenium/net/UrlCheckerTest.java[106-133]
java/src/org/openqa/selenium/net/UrlChecker.java[112-144]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new test intends to verify that interrupt status is preserved when `waitUntilUnavailable` is interrupted while blocked, but it calls `waitUntilUnavailable` when the server is not running. Because `waitUntilUnavailable` treats connection `IOException` as “unavailable” and returns immediately, the test often won’t block long enough to be interrupted and therefore won’t throw the expected `RuntimeException`.

## Issue Context
`@BeforeEach` creates `this.server` and `this.url` but does not start the server. In `UrlChecker.waitUntilUnavailable`, an `IOException` while connecting returns success immediately.

## Fix Focus Areas
- java/test/org/openqa/selenium/net/UrlCheckerTest.java[42-57]
- java/test/org/openqa/selenium/net/UrlCheckerTest.java[106-133]
- java/src/org/openqa/selenium/net/UrlChecker.java[112-163]

## Suggested fix
In `waitUntilUnavailablePreservesInterruptStatus`, start the server (and optionally call `waitUntilAvailable` to ensure it is serving HTTP 200) before invoking `waitUntilUnavailable`, and keep the server running so `waitUntilUnavailable` blocks until the interrupter interrupts the calling thread. Also ensure cleanup (joining interrupter and clearing interrupt) happens even if assertions fail (e.g., via `try/finally`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. waitUntilUnavailable missing Javadoc 📘 Rule violation ✧ Quality

Description

The modified public method waitUntilUnavailable(long timeout, TimeUnit unit, URL url) has no
Javadoc block, so its parameters and TimeoutException contract are undocumented. This violates the
requirement for complete Javadoc on changed public API methods.

Code

java/src/org/openqa/selenium/net/UrlChecker.java[R157-161]

+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      throw new RuntimeException(e);
+    } catch (ExecutionException e) {
      throw new RuntimeException(e);

Evidence
PR Compliance ID 330201 requires that every changed public method in non-test code have a Javadoc
block with descriptive text and complete tags. In UrlChecker, the public method
waitUntilUnavailable(...) has no /** ... */ comment immediately above its declaration, and the
method body was modified in this PR (catch handling), making it in-scope.
Rule 330201: Require complete Javadoc on public API methods
java/src/org/openqa/selenium/net/UrlChecker.java[108-114]
java/src/org/openqa/selenium/net/UrlChecker.java[151-162]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A changed public method `waitUntilUnavailable(...)` lacks a Javadoc block and therefore does not document purpose, `@param` tags, or the `@throws` contract.

## Issue Context
This PR modifies the behavior of `UrlChecker.waitUntilUnavailable`, so it is in-scope for the “complete Javadoc on public API methods” requirement.

## Fix Focus Areas
- java/src/org/openqa/selenium/net/UrlChecker.java[108-114]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Previous review results

Review updated until commit 1a8240e

Results up to commit f490fa3

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Interrupt test doesn’t block~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

waitUntilUnavailablePreservesInterruptStatus calls waitUntilUnavailable while the
NettyAppServer created in @BeforeEach is never started, so waitUntilUnavailable can return
immediately on IOException and never throw the expected RuntimeException. As written, the test
can fail (no exception thrown) and does not reliably validate that the interrupt status is preserved
during a blocking wait.

Code

java/test/org/openqa/selenium/net/UrlCheckerTest.java[R106-133]

+  @Test
+  void waitUntilUnavailablePreservesInterruptStatus() throws Exception {
+    Thread caller = Thread.currentThread();
+    Thread interrupter =
+        new Thread(
+            () -> {
+              try {
+                Thread.sleep(500);
+              } catch (InterruptedException ignored) {
+              }
+              caller.interrupt();
+            });
+    interrupter.start();
+
+    boolean threw = false;
+    boolean preserved = false;
+    try {
+      urlChecker.waitUntilUnavailable(10, TimeUnit.SECONDS, url);
+    } catch (RuntimeException expected) {
+      threw = true;
+      preserved = Thread.currentThread().isInterrupted();
+      Thread.interrupted();
+    }
+    interrupter.join();
+
+    assertThat(threw).isTrue();
+    assertThat(preserved).isTrue();
+  }

Evidence
The test’s server is never started, and waitUntilUnavailable returns null immediately on
IOException, so the test may never reach the interruption/exception path it asserts on.
java/test/org/openqa/selenium/net/UrlCheckerTest.java[42-57]
java/test/org/openqa/selenium/net/UrlCheckerTest.java[106-133]
java/src/org/openqa/selenium/net/UrlChecker.java[112-144]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new test intends to verify that interrupt status is preserved when `waitUntilUnavailable` is interrupted while blocked, but it calls `waitUntilUnavailable` when the server is not running. Because `waitUntilUnavailable` treats connection `IOException` as “unavailable” and returns immediately, the test often won’t block long enough to be interrupted and therefore won’t throw the expected `RuntimeException`.

## Issue Context
`@BeforeEach` creates `this.server` and `this.url` but does not start the server. In `UrlChecker.waitUntilUnavailable`, an `IOException` while connecting returns success immediately.

## Fix Focus Areas
- java/test/org/openqa/selenium/net/UrlCheckerTest.java[42-57]
- java/test/org/openqa/selenium/net/UrlCheckerTest.java[106-133]
- java/src/org/openqa/selenium/net/UrlChecker.java[112-163]

## Suggested fix
In `waitUntilUnavailablePreservesInterruptStatus`, start the server (and optionally call `waitUntilAvailable` to ensure it is serving HTTP 200) before invoking `waitUntilUnavailable`, and keep the server running so `waitUntilUnavailable` blocks until the interrupter interrupts the calling thread. Also ensure cleanup (joining interrupter and clearing interrupt) happens even if assertions fail (e.g., via `try/finally`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. waitUntilUnavailable missing Javadoc 📘 Rule violation ✧ Quality

Description

The modified public method waitUntilUnavailable(long timeout, TimeUnit unit, URL url) has no
Javadoc block, so its parameters and TimeoutException contract are undocumented. This violates the
requirement for complete Javadoc on changed public API methods.

Code

java/src/org/openqa/selenium/net/UrlChecker.java[R157-161]

+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      throw new RuntimeException(e);
+    } catch (ExecutionException e) {
      throw new RuntimeException(e);

Evidence
PR Compliance ID 330201 requires that every changed public method in non-test code have a Javadoc
block with descriptive text and complete tags. In UrlChecker, the public method
waitUntilUnavailable(...) has no /** ... */ comment immediately above its declaration, and the
method body was modified in this PR (catch handling), making it in-scope.
Rule 330201: Require complete Javadoc on public API methods
java/src/org/openqa/selenium/net/UrlChecker.java[108-114]
java/src/org/openqa/selenium/net/UrlChecker.java[151-162]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A changed public method `waitUntilUnavailable(...)` lacks a Javadoc block and therefore does not document purpose, `@param` tags, or the `@throws` contract.

## Issue Context
This PR modifies the behavior of `UrlChecker.waitUntilUnavailable`, so it is in-scope for the “complete Javadoc on public API methods” requirement.

## Fix Focus Areas
- java/src/org/openqa/selenium/net/UrlChecker.java[108-114]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-code-review · 2026-06-26T06:06:45Z

+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      throw new RuntimeException(e);
+    } catch (ExecutionException e) {
      throw new RuntimeException(e);


1. waituntilunavailable missing javadoc 📘 Rule violation ✧ Quality

The modified public method waitUntilUnavailable(long timeout, TimeUnit unit, URL url) has no Javadoc block, so its parameters and TimeoutException contract are undocumented. This violates the requirement for complete Javadoc on changed public API methods.

Agent Prompt

## Issue description A changed public method `waitUntilUnavailable(...)` lacks a Javadoc block and therefore does not document purpose, `@param` tags, or the `@throws` contract. ## Issue Context This PR modifies the behavior of `UrlChecker.waitUntilUnavailable`, so it is in-scope for the “complete Javadoc on public API methods” requirement. ## Fix Focus Areas - java/src/org/openqa/selenium/net/UrlChecker.java[108-114]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

…nterrupt test The @beforeeach created the server but did not start it, so waitUntilUnavailable returned immediately (the URL was already down) before the 500ms interrupt could fire, so the test did not exercise the interrupt path. Start the server and wait until it is available first, matching testWaitUntilUnavailableIsTimely.

vasiliy-mikhailov · 2026-07-18T07:16:14Z

Good catch. The test was creating the server in @BeforeEach but never starting it, so url was already unavailable and waitUntilUnavailable returned immediately, before the interrupt could fire, meaning the test did not actually exercise the interrupt path.

Fixed: the test now server.start() + waitUntilAvailable(...) before calling waitUntilUnavailable, and leaves the server running, so waitUntilUnavailable blocks (polling while the URL stays up) until the interrupt at 500 ms fires. This mirrors the setup of the existing testWaitUntilUnavailableIsTimely.

qodo-code-review · 2026-07-18T07:17:41Z

Code review by qodo was updated up to the latest commit 1a8240e

diemol · 2026-07-18T17:32:34Z

I'd be great if you could respect the PR template. We do allow contributors to get AI help to shape the contributor, but then this looks as AI did the whole job.

selenium-ci added the C-java Java Bindings label Jun 26, 2026

qodo-code-review Bot reviewed Jun 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Restore the interrupt status in UrlChecker.waitUntilUnavailable - #17719

Restore the interrupt status in UrlChecker.waitUntilUnavailable#17719
vasiliy-mikhailov wants to merge 2 commits into
SeleniumHQ:trunkfrom
vasiliy-mikhailov:fix/urlchecker-restore-interrupt

vasiliy-mikhailov commented Jun 26, 2026 •

edited

Loading

Uh oh!

qodo-code-review Bot commented Jun 26, 2026

Uh oh!

qodo-code-review Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

qodo-code-review Bot Jun 26, 2026

Uh oh!

Uh oh!

vasiliy-mikhailov commented Jul 18, 2026

Uh oh!

qodo-code-review Bot commented Jul 18, 2026

Uh oh!

diemol commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

vasiliy-mikhailov commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI assistance disclosure

Uh oh!

qodo-code-review Bot commented Jun 26, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Previous review results

Uh oh!

qodo-code-review Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vasiliy-mikhailov commented Jul 18, 2026

Uh oh!

qodo-code-review Bot commented Jul 18, 2026

Uh oh!

diemol commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vasiliy-mikhailov commented Jun 26, 2026 •

edited

Loading

qodo-code-review Bot commented Jun 26, 2026 •

edited

Loading