Skip to content

Handle blocked I/O of exec'd processes, and remove flaky TestExecWindowsOpenHandles#39383

Merged
cpuguy83 merged 1 commit intomoby:masterfrom
crosbymichael:exec-hang
Jul 5, 2019
Merged

Handle blocked I/O of exec'd processes, and remove flaky TestExecWindowsOpenHandles#39383
cpuguy83 merged 1 commit intomoby:masterfrom
crosbymichael:exec-hang

Conversation

@crosbymichael
Copy link
Copy Markdown
Contributor

@crosbymichael crosbymichael commented Jun 20, 2019 •
edited by thaJeztah
Loading

fixes #39326

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 crosbymichael@gmail.com

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jun 21, 2019

addresses containerd/containerd#3286 docker exec hang if earlier docker exec left a zombie process

@crosbymichael does this also relate to;

Should we have a test-case for this? (not sure if it's easy to test)

thaJeztah
thaJeztah requested changes Jun 21, 2019
View reviewed changes
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jun 21, 2019

Haven't seen this one before; https://jenkins.dockerproject.org/job/Docker-PRs-WoW-RS1/25656/console

21:06:29 FAIL: docker_cli_exec_test.go:538: DockerSuite.TestExecWindowsOpenHandles
21:06:29
21:06:29 assertion failed: 2 (int) != 1 (int)

I'll kick CI for RS1 to see if it was just flaky

@crosbymichael
Copy link
Copy Markdown
Contributor Author

crosbymichael commented Jun 21, 2019

I read through that test and its depending on the "bad" functionality, forking off the sleeps into the background and the actually cmd.exe is exiting. I think it's safe for this test to be removed with this change as well as the test is horribly racy and depending on how many seconds the sleeps wait until they exit, its bad!

@crosbymichael
Handle blocked I/O of exec'd processes
b5f2886 
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 <crosbymichael@gmail.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 21, 2019 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@52c1667). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master   #39383   +/-   ##
=========================================
  Coverage          ?   37.34%           
=========================================
  Files             ?      609           
  Lines             ?    45269           
  Branches          ?        0           
=========================================
  Hits              ?    16905           
  Misses            ?    26074           
  Partials          ?     2290

@thaJeztah thaJeztah mentioned this pull request Jul 3, 2019
Closed
thaJeztah
thaJeztah approved these changes Jul 3, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment •
edited
Loading

Choose a reason for hiding this comment

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

SGTM

@kolyshkin @dmcgowan PTAL

cpuguy83
cpuguy83 approved these changes Jul 5, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@cpuguy83
Copy link
Copy Markdown
Member

cpuguy83 commented Jul 5, 2019

I/O waiters seems like a good candidate for metrics as well.

@cpuguy83 cpuguy83 merged commit 089757d into moby:master Jul 5, 2019
@thaJeztah thaJeztah mentioned this pull request Jul 8, 2019
Merged
@kolyshkin kolyshkin mentioned this pull request Aug 7, 2019
Merged
@thaJeztah thaJeztah changed the title Handle blocked I/O of exec'd processes Handle blocked I/O of exec'd processes, and remove flaky TestExecWindowsOpenHandles Sep 3, 2019
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpuguy83 cpuguy83 cpuguy83 approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

@cpuguy83 cpuguy83

Labels

area/runtime Runtime process/cherry-picked status/2-code-review

Projects

None yet

Milestone

20.10.0

Development

Successfully merging this pull request may close these issues.

Zombie reaping not occurring as expected

4 participants

@crosbymichael @thaJeztah @cpuguy83 @GordonTheTurtle