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

Use windows api to honour timeouts (fixes #3111) #3112

Merged
merged 7 commits into from May 8, 2023

Conversation

ImogenBits
Copy link
Contributor

This fixes #3111 by using the overlapped file IO mode and associated wait windows api functions.

Signed-off-by: Imogen <59090860+ImogenBits@users.noreply.github.com>
Signed-off-by: Imogen <59090860+ImogenBits@users.noreply.github.com>
Signed-off-by: Imogen <59090860+ImogenBits@users.noreply.github.com>
Signed-off-by: Imogen <59090860+ImogenBits@users.noreply.github.com>
Copy link
Member

@milas milas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See note about ensuring we close the handle explicitly, but LGTM otherwise! Thanks for the PR!

docker/transport/npipesocket.py Outdated Show resolved Hide resolved
docker/transport/npipesocket.py Show resolved Hide resolved
Signed-off-by: Imogen <59090860+ImogenBits@users.noreply.github.com>
Signed-off-by: Imogen <59090860+ImogenBits@users.noreply.github.com>
Signed-off-by: Imogen <59090860+ImogenBits@users.noreply.github.com>
@milas milas merged commit 9cadad0 into docker:main May 8, 2023
10 checks passed
felixfontein added a commit to felixfontein/community.docker that referenced this pull request May 10, 2023
Cherry-picked from docker/docker-py@9cadad0

Co-authored-by: Imogen <59090860+ImogenBits@users.noreply.github.com>
felixfontein added a commit to ansible-collections/community.docker that referenced this pull request May 20, 2023
* socket: fix for errors on pipe close in Windows (docker/docker-py#3099)

Need to return data, not size. By returning an empty
string, EOF will be detected properly since `len()`
will be `0`.

Fixes docker/docker-py#3098.

Cherry-picked from docker/docker-py@f846232

Co-authored-by: Milas Bowman <milas.bowman@docker.com>

* socket: use poll() instead of select() except on Windows (docker/docker-py#2865)

Fixes docker/docker-py#2278, which was originally addressed in docker/docker-py#2279, but was not
properly merged. Additionally it did not address the problem
of poll not existing on Windows. This patch falls back on the
more limited select method if host system is Windows.

Cherry-picked from docker/docker-py@a02ba74

Co-authored-by: Tyler Westland <tylerofthewest@gmail.com>

* api: respect timeouts on Windows named pipes (docker/docker-py#3112)

Cherry-picked from docker/docker-py@9cadad0

Co-authored-by: Imogen <59090860+ImogenBits@users.noreply.github.com>

* Add URL to changelog.

* api: avoid socket timeouts when executing commands (docker/docker-py#3125)

Only listen to read events when polling a socket in order
to avoid incorrectly trying to read from a socket that is
not actually ready.

Cherry-picked from docker/docker-py@c5e582c

Co-authored-by: Loïc Leyendecker <loic.leyendecker@gmail.com>

---------

Co-authored-by: Milas Bowman <milas.bowman@docker.com>
Co-authored-by: Tyler Westland <tylerofthewest@gmail.com>
Co-authored-by: Imogen <59090860+ImogenBits@users.noreply.github.com>
Co-authored-by: Loïc Leyendecker <loic.leyendecker@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timeouts don't work on windows
2 participants