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.

Sign up for GitHub

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

Jump to bottom

Fix docker start blocking on signal handling #2990

Merged
merged 1 commit into from
May 7, 2021
Merged

Fix docker start blocking on signal handling #2990

merged 1 commit into from
May 7, 2021

Conversation

cpuguy83
Copy link
Collaborator

@cpuguy83 cpuguy83 commented Mar 2, 2021 •
edited by thaJeztah
Loading

introduced in #2929

We refactorted ForwardAllSignals so it blocks but did not update the
call in start to call it in a goroutine.

Closes (maybe) #2987

@cpuguy83
Fix docker start blocking on signal handling
e1a7517 
We refactorted `ForwardAllSignals` so it blocks but did not update the
call in `start` to call it in a goroutine.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@tiborvass tiborvass mentioned this pull request Mar 2, 2021
Merged
@thaJeztah
Copy link
Member

Closes (maybe) #2987

We discussed this internally, and to be on the safe side, we want to do a 20.10.5 "hotfix" with the revert today (which unblocks Docker Desktop to include the 20.10.x updates); this gives us some time to verify the fixes, and then we can "un-revert" the changes (and include the fixes) for 20.10.6.
There are some important fixes pending (the docker logs EOF fix, as well as the libnetwork fixes); once those are merged, we should do the 20.10.6 release.

thaJeztah
thaJeztah approved these changes Mar 2, 2021
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member

@tonistiigi LGTY?

@thaJeztah thaJeztah added this to the 21.xx milestone Mar 2, 2021
@cpuguy83
Copy link
Collaborator Author

cpuguy83 commented Apr 7, 2021

Ping.
Master is still broken due to this.

@thaJeztah
Copy link
Member

🙈 thought this was merged already, but I was waiting for Tõnis at the time.

Let me merge

@thaJeztah thaJeztah merged commit 19c5aab into docker:master May 7, 2021
@thaJeztah thaJeztah mentioned this pull request May 25, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

@tiborvass tiborvass tiborvass approved these changes

@tonistiigi tonistiigi Awaiting requested review from tonistiigi

Assignees
No one assigned
Labels
process/cherry-picked status/2-code-review
Projects
None yet
Milestone
23.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@cpuguy83 @thaJeztah @tiborvass