Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docker exec hangs indefinitely when reading from stdin #2094

Open
nicks opened this issue Apr 25, 2022 · 9 comments · May be fixed by #7896
Open

docker exec hangs indefinitely when reading from stdin #2094

nicks opened this issue Apr 25, 2022 · 9 comments · May be fixed by #7896
Assignees
Labels
kind/bug Something isn't working parity/project Feature is available from other projects platform/windows runtime/moby

Comments

@nicks
Copy link

nicks commented Apr 25, 2022

Actual Behavior

When using the Docker CLI with rancher desktop, docker exec commands that read from stdin hang indefinitely. Reproduced on multiple Windows machines. Haven't tested on other operating systems.

Steps to Reproduce

docker run -d --name busybox busybox sleep 1000
echo hi | docker exec -i busybox cat -

Result

The CLI never exits

Expected Behavior

The CLI should exit

This works correctly with Docker Desktop

Additional Information

This is a pared down repro case of a bigger downstream bug that someone reported to my project when using it with Rancher Desktop:
tilt-dev/tilt#5730

Rancher Desktop Version

1.2.1

Rancher Desktop K8s Version

1.22.7

Which container runtime are you using?

moby (docker cli)

What operating system are you using?

Windows

Operating System / Build Version

Windows 11 Pro

What CPU architecture are you using?

x64

Linux only: what package format did you use to install Rancher Desktop?

No response

Windows User Only

No

@chrmarti
Copy link

Similarly on Windows 10: cat test.tar | docker exec -i <container id> tar t hangs. (Reported in microsoft/vscode-remote-release#6571.)

@gunamata gunamata added the triage/next-candidate Discuss if it should be moved to "Next" milestone label May 10, 2022
@mook-as mook-as self-assigned this May 10, 2022
@gaktive gaktive added this to the Next milestone May 10, 2022
@gaktive gaktive removed the triage/next-candidate Discuss if it should be moved to "Next" milestone label May 10, 2022
@jandubois jandubois modified the milestones: Next, Later May 20, 2022
@jandubois jandubois modified the milestones: Next, Later Jul 14, 2022
@gaktive gaktive modified the milestones: Next, Later Sep 13, 2022
@R-Beck-2020
Copy link

Please fix this, I have had to revert to Docker Desktop whilst this remains an issue.

@mwisnicki
Copy link

Same here.

Additionally when executed from WSL there is no hang but there is also no output.

@jandubois jandubois added the triage/need-to-repro Needs to be reproduced by dev team label Jun 12, 2023
@mwisnicki
Copy link

mwisnicki commented Jun 12, 2023

Simplified repro steps:

echo ok | docker run -i --rm alpine cat

@jandubois jandubois modified the milestones: Later, 1.10 Jun 13, 2023
@cbast
Copy link

cbast commented Jun 28, 2023

Simplified repro steps:

echo ok | docker run -i --rm alpine cat

I get the same error. I'm not sure if it helps the investigation, but I get different behaviors depending on which WSL image I'm running from.

  • Using the "Ubuntu" WSL image: the logs are not displayed on the terminal (as you mentionned).
  • Using the "rancher-desktop" WSL image: the logs are displayed on the terminal.

In both cases though, the container does execute, it is just the output that is not available.

Is it the case for you too?

In docker.log (Rancher Desktop -> Troubleshooting -> Show Logs) I have the following error every time I run a container:
time="2023-06-28T15:28:48.771893386Z" level=error msg="attach failed with error: error attaching stdout stream: write unix /mnt/wsl/rancher-desktop/run/docker.sock->@: write: broken pipe"

In "Preferences -> WSL" I checked "Ubuntu" to expose docker's socket.

@jandubois jandubois added the parity/project Feature is available from other projects label Jul 25, 2023
@mook-as
Copy link
Contributor

mook-as commented Aug 24, 2023

Status update, since I've been poking it for a few days:

  • First attempt: make wsl-helper docker-proxy start also listen on a Unix socket (in the WSL VM), and talk to that.
    • This exposed a bug somewhere in our pipe implementation; if we move the Close() calls to a timer, it seems to help not lose the last bits of the connection. (But this is obviously the opposite problem.)
  • Hacking up the docker CLI to talk to vsock directly (without going through \\.\pipe\docker_engine) seems to avoid the bug; this seems to indicate that something with wrong with the reverse proxy. (We need to use the reverse proxy to fix volume mounts, but the test case doesn't involve volume mounts.)

So this seems to indicate that when the VM -> proxy side of the connection closes, the proxy -> CLI side is still left open.


Doing some more digging, it's something to do with how we deal with Windows named pipes; if I made a dumb pipe between vsock<->npipe, that breaks too. So at least that means we don't have to worry (yet) about the proxy part. I see that the documentation for winio#PipeConfig notes that CloseWrite() is not implemented for byte mode; I wonder if that's related. (Turning on message mode breaks things even more, though; that makes sense as that doesn't mesh with how TCP &c work.)

@jandubois jandubois removed the triage/need-to-repro Needs to be reproduced by dev team label Aug 24, 2023
@mook-as
Copy link
Contributor

mook-as commented Sep 12, 2023

I think I'm getting a bit stuck:

@jandubois
Copy link
Member

Investigation is stalled; moving back to Todo

@jandubois jandubois modified the milestones: 1.11, 1.12 Sep 26, 2023
@jandubois jandubois assigned Nino-K and unassigned mook-as Nov 2, 2023
@jandubois jandubois removed the priority/1 Work should be fixed for next release label Nov 16, 2023
@gaktive gaktive modified the milestones: 1.12, 1.13 Nov 21, 2023
@jandubois jandubois removed this from the 1.13 milestone Jan 29, 2024
bcxpro added a commit to bcxpro/rancher-desktop that referenced this issue Dec 8, 2024
This commit fixes the improper handling of half-closed connection behavior when attaching stdin to a container.
The proper approach requires:
- The docker client receives a half-close of the connection from client to daemon (stdin)
- The connection from daemon to client (stdout, stderr) remains open until no more data is to be received

The first part of the issue is resolved by enabling MessageMode on Windows named pipes.
This ensures that when stdin ends, an EOF is received on the reader, allowing proper half-closing of the connection.

The previous implementation used stdlib's httputil.DockerProxy to proxy from Windows named pipe to WSL hvsock.
This proxy does not use CloseWrite on hijacked connections, preventing proper handling of this use case.
By using Close() instead of CloseWrite(), both connection directions are closed simultaneously,
causing the client to miss pending stdout/stderr content after stdin EOF.

This commit introduces a simpler custom `util.RProxy` to replace httputil.DockerProxy, implementing proper handling
of half-closed connections with explicit support for:
- HTTP protocol upgrades
- Half-close TCP connection management
- Precise stream handling for Docker API interactions

Closes rancher-sandbox#2094

Signed-off-by: Carlos Barcenilla <[email protected]>
bcxpro added a commit to bcxpro/rancher-desktop that referenced this issue Dec 8, 2024
This commit fixes the improper handling of half-closed connection behavior when attaching stdin to a container.
The proper approach requires:
- The docker client receives a half-close of the connection from client to daemon (stdin)
- The connection from daemon to client (stdout, stderr) remains open until no more data is to be received

The first part of the issue is resolved by enabling MessageMode on Windows named pipes.
This ensures that when stdin ends, an EOF is received on the reader, allowing proper half-closing of the connection.

The previous implementation used stdlib's httputil.DockerProxy to proxy from Windows named pipe to WSL hvsock.
This proxy does not use CloseWrite on hijacked connections, preventing proper handling of this use case.
By using Close() instead of CloseWrite(), both connection directions are closed simultaneously,
causing the client to miss pending stdout/stderr content after stdin EOF.

This commit introduces a simpler custom `util.RProxy` to replace httputil.DockerProxy, implementing proper handling
of half-closed connections with explicit support for:
- HTTP protocol upgrades
- Half-close TCP connection management
- Precise stream handling for Docker API interactions

Closes rancher-sandbox#2094

Signed-off-by: Carlos Barcenilla <[email protected]>
@bcxpro
Copy link

bcxpro commented Dec 8, 2024

Hi

I just opened This is fixed in PR #7896 with a proposed solution of this problem. Please take a look at it.

I started troubleshooting it because I wanted to run kind on Windows. When creating a cluster kind sends configuration files over stdin. Then it failed. I tested it with my proposed solution and now everything works.

I implemented a simpler and more naive version of the Reverse proxy that handles everything needed to poxy docker connections. However, I am not a seasoned go developer so my implementation may be improved.

bcxpro added a commit to bcxpro/rancher-desktop that referenced this issue Dec 8, 2024
This commit fixes the improper handling of half-closed connection behavior when attaching stdin to a container.
The proper approach requires:
- The docker client receives a half-close of the connection from client to daemon (stdin)
- The connection from daemon to client (stdout, stderr) remains open until no more data is to be received

The first part of the issue is resolved by enabling MessageMode on Windows named pipes.
This ensures that when stdin ends, an EOF is received on the reader, allowing proper half-closing of the connection.

The previous implementation used stdlib's httputil.DockerProxy to proxy from Windows named pipe to WSL hvsock.
This proxy does not use CloseWrite on hijacked connections, preventing proper handling of this use case.
By using Close() instead of CloseWrite(), both connection directions are closed simultaneously,
causing the client to miss pending stdout/stderr content after stdin EOF.

This commit introduces a simpler custom `util.RProxy` to replace httputil.DockerProxy, implementing proper handling
of half-closed connections with explicit support for:
- HTTP protocol upgrades
- Half-close TCP connection management
- Precise stream handling for Docker API interactions

Closes rancher-sandbox#2094

Signed-off-by: Carlos Barcenilla <[email protected]>
bcxpro added a commit to bcxpro/rancher-desktop that referenced this issue Dec 8, 2024
This commit fixes the improper handling of half-closed connection behavior when attaching stdin to a container.
The proper approach requires:
- The docker client receives a half-close of the connection from client to daemon (stdin)
- The connection from daemon to client (stdout, stderr) remains open until no more data is to be received

The first part of the issue is resolved by enabling MessageMode on Windows named pipes.
This ensures that when stdin ends, an EOF is received on the reader, allowing proper half-closing of the connection.

The previous implementation used stdlib's httputil.DockerProxy to proxy from Windows named pipe to WSL hvsock.
This proxy does not use CloseWrite on hijacked connections, preventing proper handling of this use case.
By using Close() instead of CloseWrite(), both connection directions are closed simultaneously,
causing the client to miss pending stdout/stderr content after stdin EOF.

This commit introduces a simpler custom `util.RProxy` to replace httputil.DockerProxy, implementing proper handling
of half-closed connections with explicit support for:
- HTTP protocol upgrades
- Half-close TCP connection management
- Precise stream handling for Docker API interactions

Closes rancher-sandbox#2094

Signed-off-by: Carlos Barcenilla <[email protected]>
bcxpro added a commit to bcxpro/rancher-desktop that referenced this issue Dec 8, 2024
This commit fixes the improper handling of half-closed connection behavior when attaching stdin to a container.
The proper approach requires:
- The docker client receives a half-close of the connection from client to daemon (stdin)
- The connection from daemon to client (stdout, stderr) remains open until no more data is to be received

The first part of the issue is resolved by enabling MessageMode on Windows named pipes.
This ensures that when stdin ends, an EOF is received on the reader, allowing proper half-closing of the connection.

The previous implementation used stdlib's httputil.DockerProxy to proxy from Windows named pipe to WSL hvsock.
This proxy does not use CloseWrite on hijacked connections, preventing proper handling of this use case.
By using Close() instead of CloseWrite(), both connection directions are closed simultaneously,
causing the client to miss pending stdout/stderr content after stdin EOF.

This commit introduces a simpler custom `util.ReverseProxy` to replace httputil.DockerProxy, implementing proper handling
of half-closed connections with explicit support for:
- HTTP protocol upgrades
- Half-close TCP connection management
- Precise stream handling for Docker API interactions

Closes rancher-sandbox#2094

Signed-off-by: Carlos Barcenilla <[email protected]>
bcxpro added a commit to bcxpro/rancher-desktop that referenced this issue Dec 10, 2024
This commit fixes the improper handling of half-closed connection behavior when attaching stdin to a container.
The proper approach requires:
- The docker client receives a half-close of the connection from client to daemon (stdin)
- The connection from daemon to client (stdout, stderr) remains open until no more data is to be received

The first part of the issue is resolved by enabling MessageMode on Windows named pipes.
This ensures that when stdin ends, an EOF is received on the reader, allowing proper half-closing of the connection.

The previous implementation used stdlib's httputil.DockerProxy to proxy from Windows named pipe to WSL hvsock.
This proxy does not use CloseWrite on hijacked connections, preventing proper handling of this use case.
By using Close() instead of CloseWrite(), both connection directions are closed simultaneously,
causing the client to miss pending stdout/stderr content after stdin EOF.

This commit introduces a simpler custom `util.ReverseProxy` to replace httputil.DockerProxy, implementing proper handling
of half-closed connections with explicit support for:
- HTTP protocol upgrades
- Half-close TCP connection management
- Precise stream handling for Docker API interactions

Closes rancher-sandbox#2094

Signed-off-by: Carlos Barcenilla <[email protected]>
bcxpro added a commit to bcxpro/rancher-desktop that referenced this issue Dec 10, 2024
This commit fixes the improper handling of half-closed connection behavior when attaching stdin to a container.
The proper approach requires:
- The docker client receives a half-close of the connection from client to daemon (stdin)
- The connection from daemon to client (stdout, stderr) remains open until no more data is to be received

The first part of the issue is resolved by enabling MessageMode on Windows named pipes.
This ensures that when stdin ends, an EOF is received on the reader, allowing proper half-closing of the connection.

The previous implementation used stdlib's httputil.DockerProxy to proxy from Windows named pipe to WSL hvsock.
This proxy does not use CloseWrite on hijacked connections, preventing proper handling of this use case.
By using Close() instead of CloseWrite(), both connection directions are closed simultaneously,
causing the client to miss pending stdout/stderr content after stdin EOF.

This commit introduces a simpler custom `util.ReverseProxy` to replace httputil.DockerProxy, implementing proper handling
of half-closed connections with explicit support for:
- HTTP protocol upgrades
- Half-close TCP connection management
- Precise stream handling for Docker API interactions

Closes rancher-sandbox#2094

Signed-off-by: Carlos Barcenilla <[email protected]>
bcxpro added a commit to bcxpro/rancher-desktop that referenced this issue Dec 11, 2024
This commit fixes the improper handling of half-closed connection behavior when attaching stdin to a container.
The proper approach requires:
- The docker client receives a half-close of the connection from client to daemon (stdin)
- The connection from daemon to client (stdout, stderr) remains open until no more data is to be received

The first part of the issue is resolved by enabling MessageMode on Windows named pipes.
This ensures that when stdin ends, an EOF is received on the reader, allowing proper half-closing of the connection.

The previous implementation used stdlib's httputil.DockerProxy to proxy from Windows named pipe to WSL hvsock.
This proxy does not use CloseWrite on hijacked connections, preventing proper handling of this use case.
By using Close() instead of CloseWrite(), both connection directions are closed simultaneously,
causing the client to miss pending stdout/stderr content after stdin EOF.

This commit introduces a simpler custom `util.ReverseProxy` to replace httputil.DockerProxy, implementing proper handling
of half-closed connections with explicit support for:
- HTTP protocol upgrades
- Half-close TCP connection management
- Precise stream handling for Docker API interactions

Closes rancher-sandbox#2094

Signed-off-by: Carlos Barcenilla <[email protected]>
bcxpro added a commit to bcxpro/rancher-desktop that referenced this issue Dec 28, 2024
This commit fixes the improper handling of half-closed connection behavior when attaching stdin to a container.
The proper approach requires:
- The docker client receives a half-close of the connection from client to daemon (stdin)
- The connection from daemon to client (stdout, stderr) remains open until no more data is to be received

The first part of the issue is resolved by enabling MessageMode on Windows named pipes.
This ensures that when stdin ends, an EOF is received on the reader, allowing proper half-closing of the connection.

The previous implementation used stdlib's httputil.DockerProxy to proxy from Windows named pipe to WSL hvsock.
This proxy does not use CloseWrite on hijacked connections, preventing proper handling of this use case.
By using Close() instead of CloseWrite(), both connection directions are closed simultaneously,
causing the client to miss pending stdout/stderr content after stdin EOF.

This commit introduces a simpler custom `util.ReverseProxy` to replace httputil.DockerProxy, implementing proper handling
of half-closed connections with explicit support for:
- HTTP protocol upgrades
- Half-close TCP connection management
- Precise stream handling for Docker API interactions

Closes rancher-sandbox#2094

Signed-off-by: Carlos Barcenilla <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working parity/project Feature is available from other projects platform/windows runtime/moby
Projects
None yet
Development

Successfully merging a pull request may close this issue.