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

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

Merged
merged 1 commit into from
Sep 4, 2019
Merged

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

merged 1 commit into from
Sep 4, 2019

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Aug 6, 2019 •
edited
Loading

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 #39383)

@kolyshkin
daemon/ProcessEvent: make sure to cancel the contexts
53cbf17 
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 <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor Author

@crosbymichael PTAL

@kolyshkin kolyshkin mentioned this pull request Aug 7, 2019
Merged
thaJeztah
thaJeztah approved these changes Aug 12, 2019
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

ping @crosbymichael @dmcgowan PTAL

@thaJeztah
Copy link
Member

ping @crosbymichael @dmcgowan PTAL

@thaJeztah thaJeztah assigned dmcgowan and crosbymichael and unassigned thaJeztah Sep 3, 2019
cpuguy83
cpuguy83 approved these changes Sep 4, 2019
View reviewed changes
Copy link
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 cpuguy83 merged commit 00c9e77 into moby:master Sep 4, 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

@crosbymichael crosbymichael Awaiting requested review from crosbymichael

Assignees

@dmcgowan dmcgowan

@crosbymichael crosbymichael

Labels
area/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.
6 participants
@kolyshkin @thaJeztah @cpuguy83 @dmcgowan @crosbymichael @GordonTheTurtle