Skip to content

context.WithTimeout: do call the cancel func#36920

Merged
thaJeztah merged 1 commit intomoby:masterfrom
kolyshkin:cancel-func
Apr 23, 2018
Merged

context.WithTimeout: do call the cancel func#36920
thaJeztah merged 1 commit intomoby:masterfrom
kolyshkin:cancel-func

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin commented Apr 20, 2018

govet complains (when using standard "context" package in #36904):

the cancel function returned by context.WithTimeout should be called,
not discarded, to avoid a context leak (vet)

fix it.

@kolyshkin
context.WithTimeout: do call the cancel func
05e2f7e 
govet complains (when using standard "context" package):

> the cancel function returned by context.WithTimeout should be called,
> not discarded, to avoid a context leak (vet)

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin mentioned this pull request Apr 20, 2018
Merged
thaJeztah
thaJeztah approved these changes Apr 20, 2018
View reviewed changes
Copy link
Copy Markdown
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

ping @tonistiigi @aaronlehmann @cpuguy83

cpuguy83
cpuguy83 approved these changes Apr 20, 2018
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

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Apr 20, 2018

Failing on flaky test #36501

20:51:14 FAIL: docker_api_swarm_service_test.go:33: DockerSwarmSuite.TestAPIServiceUpdatePort
20:51:14 
20:51:14 [d844a9c891505] waiting for daemon to start
20:51:14 [d844a9c891505] daemon started
20:51:14 
20:51:14 docker_api_swarm_service_test.go:39:
20:51:14     waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1)
20:51:14 docker_utils_test.go:435:
20:51:14     c.Assert(v, checker, args...)
20:51:14 ... obtained int = 0
20:51:14 ... expected int = 1
20:51:14 
20:51:14 [d844a9c891505] exiting daemon
20:51:26

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 23, 2018 •
edited
Loading

Codecov Report

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

@@            Coverage Diff            @@
##             master   #36920   +/-   ##
=========================================
  Coverage          ?   35.42%           
=========================================
  Files             ?      614           
  Lines             ?    46050           
  Branches          ?        0           
=========================================
  Hits              ?    16315           
  Misses            ?    27582           
  Partials          ?     2153

@tonistiigi
Copy link
Copy Markdown
Member

tonistiigi commented Apr 23, 2018

LGTM

@thaJeztah thaJeztah merged commit 20b524b into moby:master Apr 23, 2018
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

No one assigned

Labels

status/4-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@kolyshkin @thaJeztah @tonistiigi @cpuguy83 @vdemeester @yongtang @GordonTheTurtle