Skip to content

Connect to the Windows named pipe without an existence pre-check - #12129

Open
mertakdut wants to merge 1 commit into
testcontainers:mainfrom
mertakdut:npipe-busy-pipe
Open

mertakdut wants to merge 1 commit into
testcontainers:mainfrom
mertakdut:npipe-busy-pipe

Conversation

@mertakdut

@mertakdut mertakdut commented Oct 23, 2026 •

Copy link
Copy Markdown

Fixes #12128

On Windows, test JVMs running in parallel against Docker Desktop intermittently find no Docker environment: one JVM fails with "Could not find a valid Docker environment … Attempted configurations were:" and an empty list, and every later attempt in that JVM fails fast. Docker Desktop is running and a re-run passes.

DockerClientProviderStrategy.test() checks new File(dockerHost.getPath()).exists() before it connects. On a named pipe, Windows answers that question by opening an instance of the pipe, and while every instance is taken (by the other JVMs' Docker traffic or their own checks) the open fails with ERROR_PIPE_BUSY, which the JDK reports as "does not exist". The check returns false without recording a reason, and with no other applicable strategy on Windows FAIL_FAST_ALWAYS is set. Measured in one JVM over 50 rounds, simultaneous File.exists("//./pipe/docker_engine") calls answer false 21 % of the time with 4 callers and 50 % with 16; the issue has the full mechanism and measurements.

The change skips the existence pre-check for the npipe scheme and lets the connection attempt decide: NamedPipeSocket.connect already waits on a busy pipe (docker-java#1418) and fails on an absent one. The unix branch is unchanged.

Behaviour change for an absent pipe (Docker Desktop stopped): it is detected after ten seconds, the floor NamedPipeSocket.connect puts on its own wait, or after the client ping timeout when that is lower, instead of at once. It is logged at warn level, as every connection failure in test() is, and the list of attempted configurations stays empty as before; recording the reason there would need the catch shared by the tcp, unix and npipe branches to append to the list too, which this pull request leaves for you to weigh.

How it was verified, on Windows 11 with Docker Desktop 4.96.3:

  • DockerClientProviderStrategyTest (new, Windows only) creates a single-instance named pipe and holds it with one client, which makes it busy without a race. File.exists() answers false on it, and test() must pass once the holder releases it within the wait: it fails on main and passes with the change. A second case checks that test() still answers false for a pipe that does not exist; it passes on both, and takes ten seconds with the change.
  • The org.testcontainers.dockerclient package and DockerClientFactoryTest pass against Docker Desktop with the change, except DockerClientConfigUtilsTest.getDefaultGateway(), which hangs on this machine reading an image-inspect response with and without the change.
  • ./gradlew checkstyleMain checkstyleTest spotlessApply is clean.

Related: #5017, which describes the same symptom on Windows with parallel tests and works around it by resetting FAIL_FAST_ALWAYS through reflection. A downstream project carries this change as a local client provider strategy until a release ships it.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Docker connection checks on Windows so an active named pipe is not mistakenly treated as unavailable. Connections can proceed once the pipe becomes available, while connections to nonexistent pipes continue to fail. This helps make Docker availability detection more reliable for Windows users.

@mertakdut
mertakdut requested a review from a team as a code owner October 23, 2026 21:49
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c0ba61ed-f523-4af0-8cf4-aec12ce35132
📥 Commits

Reviewing files that changed from the base of the PR and between 10b3159 and e26975a.

📒 Files selected for processing: 2
  • core/src/main/java/org/testcontainers/dockerclient/DockerClientProviderStrategy.java
  • core/src/test/java/org/testcontainers/dockerclient/DockerClientProviderStrategyTest.java

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

For npipe Docker hosts, the provider strategy skips the socket-path existence check and tests availability through NamedPipeSocket. Windows-only tests cover busy and nonexistent named pipes.

Changes: Windows named-pipe detection

Layer / File(s) Summary
Named-pipe check and validation
core/src/main/java/org/testcontainers/dockerclient/DockerClientProviderStrategy.java, core/src/test/java/org/testcontainers/dockerclient/DockerClientProviderStrategyTest.java
The strategy checks socket-path existence only for unix hosts. Windows tests verify that a busy named pipe can connect after it becomes available and that a nonexistent pipe returns false. Test helpers create named pipes through JNA.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: bsideup

Merge Risk: ⚪ Minimal · up to e2697

The Windows named-pipe change is ready to merge after normal checks; no unresolved behavior issue was established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: removing the existence pre-check before connecting to a Windows named pipe.
Linked Issues check Passed Issue #12128 requires skipping the File.exists() pre-check for npipe, preserving the unix check, returning false after connection failure, and adding a Windows busy-pipe test. `DockerClientPro…
Out of Scope Changes check Passed The pull request changes only the strategy check and adds focused Windows tests for busy and nonexistent named pipes. The comments and native test helpers support issue #12128. No unrelated production…
Description check Passed The pull request description is complete. It identifies issue #12128, describes the Windows named-pipe failure, explains the code change and behavior impact, documents verification results and limitat…
Full details: Linked Issues check

Explanation

Issue #12128 requires skipping the File.exists() pre-check for npipe, preserving the unix check, returning false after connection failure, and adding a Windows busy-pipe test. DockerClientProviderStrategy.test() now applies the existence check only to unix; npipe uses NamedPipeSocket.connect through the existing connection path and the existing catch returns false. The new Windows-only test holds a single-instance named pipe, verifies File.exists() returns false while it is busy, releases the holder, and asserts that test() succeeds. A second test covers a nonexistent pipe.

Full details: Out of Scope Changes check

Explanation

The pull request changes only the strategy check and adds focused Windows tests for busy and nonexistent named pipes. The comments and native test helpers support issue #12128. No unrelated production behavior or unrelated tests are changed.

Full details: Description check

Explanation

The pull request description is complete. It identifies issue #12128, describes the Windows named-pipe failure, explains the code change and behavior impact, documents verification results and limitations, and references related issue #5017.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: DockerClientProviderStrategy.test() reports a busy Windows named pipe as absent, so parallel test JVMs intermittently find no Docker environment

1 participant