Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

[19.03 backport] Handle blocked I/O of exec'd processes #296

Merged
merged 2 commits into from
Sep 23, 2019
Merged

[19.03 backport] Handle blocked I/O of exec'd processes #296

merged 2 commits into from
Sep 23, 2019

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jul 8, 2019 •
edited
Loading

backport of moby#39383 and moby#39677
fixes moby#39326

moby#39383 Handle blocked I/O of exec'd processes

This is the second part to containerd/containerd#3361 and will help process delete not block forever when the process exists but the I/O was inherited by a subprocess that lives on.

moby#39677 daemon/ProcessEvent: make sure to cancel the contexts:

Reported by govet linter:

daemon/monitor.go:57:9: lostcancel: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak (govet)
			ctx, _ := context.WithTimeout(context.Background(), 2*time.Second)
			     ^
daemon/monitor.go:128:9: lostcancel: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak (govet)
			ctx, _ := context.WithTimeout(context.Background(), 2*time.Second)
			     ^

Fixes: b5f2886 ("Handle blocked I/O of exec'd processes", PR moby#39383)

@thaJeztah thaJeztah added this to the 19.03.0 milestone Jul 8, 2019
@thaJeztah
Copy link
Member Author

Looks like the RS1 machines have issues again

12:54:02 FAIL: check_test.go:107: DockerSuite.TearDownTest
12:54:02 
12:54:02 assertion failed: error is not nil: Error response from daemon: container 0e572cda3c5e3ddfc5fa1e6c2444dcdcc414228053757eef83b2d3370405dd81: driver "windowsfilter" failed to remove root filesystem: failed to detach VHD: invalid argument: rename D:\CI\CI-18daa443c6\daemon\windowsfilter\0e572cda3c5e3ddfc5fa1e6c2444dcdcc414228053757eef83b2d3370405dd81 D:\CI\CI-18daa443c6\daemon\windowsfilter\0e572cda3c5e3ddfc5fa1e6c2444dcdcc414228053757eef83b2d3370405dd81-removing: Access is denied.: failed to remove 0e572cda3c5e3ddfc5fa1e6c2444dcdcc414228053757eef83b2d3370405dd81
12:54:02 
12:54:02 ----------------------------------------------------------------------
12:54:02 PANIC: docker_cli_run_test.go:4152: DockerSuite.TestRunCredentialSpecFailures
12:54:02 
12:54:02 ... Panic: Fixture has panicked (see related PANIC)

@thaJeztah thaJeztah modified the milestones: 19.03.0, 19.03.1, 19.03.2 Jul 23, 2019
kolyshkin
kolyshkin approved these changes Aug 7, 2019
View reviewed changes
Copy link

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

@kolyshkin
Copy link

In fact, it make sense to also pick up moby#39677 here

@thaJeztah thaJeztah modified the milestones: 19.03.2, 19.03.3 Aug 30, 2019
@thaJeztah thaJeztah force-pushed the 19.03_backport_exec_hang branch from 18daa44 to 9f163a7 Compare September 4, 2019 20:59
@thaJeztah
Copy link
Member Author

@kolyshkin I added moby#39677

@thaJeztah thaJeztah force-pushed the 19.03_backport_exec_hang branch 2 times, most recently from 1ae7c67 to 51c3cac Compare September 12, 2019 08:14
@thaJeztah thaJeztah force-pushed the 19.03_backport_exec_hang branch from 51c3cac to a92dbea Compare September 20, 2019 07:27
crosbymichael and others added 2 commits September 20, 2019 19:10
@crosbymichael @thaJeztah
Handle blocked I/O of exec'd processes
34b31d0 
This is the second part to
containerd/containerd#3361 and will help process
delete not block forever when the process exists but the I/O was
inherited by a subprocess that lives on.

Signed-off-by: Michael Crosby <[email protected]>
(cherry picked from commit b5f2886)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@kolyshkin @thaJeztah
daemon/ProcessEvent: make sure to cancel the contexts
e489130 
Reported by govet linter:

> daemon/monitor.go:57:9: lostcancel: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak (govet)
> 			ctx, _ := context.WithTimeout(context.Background(), 2*time.Second)
> 			     ^
> daemon/monitor.go:128:9: lostcancel: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak (govet)
> 			ctx, _ := context.WithTimeout(context.Background(), 2*time.Second)
> 			     ^

Fixes: b5f288 ("Handle blocked I/O of exec'd processes")
Signed-off-by: Kir Kolyshkin <[email protected]>
(cherry picked from commit 53cbf17)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the 19.03_backport_exec_hang branch from a92dbea to e489130 Compare September 20, 2019 17:11
andrewhsu
andrewhsu approved these changes Sep 23, 2019
View reviewed changes
Copy link

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

SGTM

@andrewhsu andrewhsu merged commit 9a9ff44 into docker-archive:19.03 Sep 23, 2019
@thaJeztah thaJeztah deleted the 19.03_backport_exec_hang branch September 23, 2019 17:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kolyshkin kolyshkin kolyshkin approved these changes

@andrewhsu andrewhsu andrewhsu approved these changes

Assignees
No one assigned
Labels
impact/changelog
Projects
None yet
Milestone
19.03.3
Development

Successfully merging this pull request may close these issues.
4 participants
@thaJeztah @kolyshkin @andrewhsu @crosbymichael