Skip to content

runtime/v1/linux/proc/io: io race#3153

Merged
crosbymichael merged 2 commits intocontainerd:masterfrom
thepwagner:issue-3118
Apr 1, 2019
Merged

runtime/v1/linux/proc/io: io race#3153
crosbymichael merged 2 commits intocontainerd:masterfrom
thepwagner:issue-3118

Conversation

@thepwagner
Copy link
Copy Markdown
Contributor

This resolves #3118 by introducing synchronization to the wc.Close() calls.

  • log the IO error at Warn level
  • fix the IO error by waiting for the last WriteCloser to Close()

@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 1, 2019

Codecov Report

Merging #3153 into master will decrease coverage by 4.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3153      +/-   ##
==========================================
- Coverage   49.25%   45.16%   -4.09%     
==========================================
  Files         100      111      +11     
  Lines        9415    11962    +2547     
==========================================
+ Hits         4637     5403     +766     
- Misses       3955     5727    +1772     
- Partials      823      832       +9
Flag Coverage Δ
#linux 49.25% <ø> (ø) ⬆️
#windows 40.49% <ø> (?)
Impacted Files Coverage Δ
snapshots/native/native.go 43.04% <0%> (-9.99%) ⬇️
metadata/snapshot.go 45.8% <0%> (-8.96%) ⬇️
archive/tar.go 43.79% <0%> (-7.07%) ⬇️
metadata/containers.go 47.97% <0%> (-6.62%) ⬇️
content/local/writer.go 57.84% <0%> (-6.36%) ⬇️
content/local/store.go 48.51% <0%> (-5.03%) ⬇️
metadata/images.go 57.57% <0%> (-4.99%) ⬇️
archive/tar_opts.go 28.57% <0%> (-4.77%) ⬇️
archive/compression/compression.go 58.69% <0%> (-4.7%) ⬇️
metadata/buckets.go 56.33% <0%> (-4.6%) ⬇️
... and 61 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d0a06d...ae04c16. Read the comment docs.

Copy link
Copy Markdown
Member

@estesp estesp 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 @crosbymichael

@crosbymichael
Copy link
Copy Markdown
Member

LGTM

@crosbymichael crosbymichael merged commit 341b99d into containerd:master Apr 1, 2019
@thaJeztah
Copy link
Copy Markdown
Member

do we need to backport this to the 1.1/1.2 release branches?

@crosbymichael
Copy link
Copy Markdown
Member

@thaJeztah yes

@thaJeztah
Copy link
Copy Markdown
Member

I'll do a quick cherry-pick 👍

@thaJeztah
Copy link
Copy Markdown
Member

1.2 Backport: #3154

looks like the code moved around between 1.1 and 1.2, so the 1.1 backport took slightly longer; #3156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

runtime/v1/linux/proc copyPipes with sameFile race

5 participants