containerd / go-runc

runc bindings for Go
Apache License 2.0
161 stars 71 forks source link

Fix data race in use of cmd output buffers. #58

Closed sipsma closed 4 years ago

sipsma commented 4 years ago

The bytes.Buffers used to store the output of subprocesses are managed with a sync.Pool. However, before this change, they were being returned to the Pool while the slice underlying the Buffer was still in use by caller methods. If that Buffer was then used by another caller, reads and writes to the underlying slice had the potential race with one another (which is detectable by running with the "-race" flag enabled in a binary and/or a test).

This change puts the responsibility on the caller of cmdOutput to put the buffer back in the pool once it is done reading data from it, ensuring no two methods use its slice at the same time.

Signed-off-by: Erik Sipsma sipsma@amazon.com

Fixes #57

codecov-io commented 4 years ago

Codecov Report

Merging #58 into master will increase coverage by 7.39%. The diff coverage is 17.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #58      +/-   ##
==========================================
+ Coverage    6.03%   13.43%   +7.39%     
==========================================
  Files           7        7              
  Lines         646      655       +9     
==========================================
+ Hits           39       88      +49     
+ Misses        600      544      -56     
- Partials        7       23      +16
Impacted Files Coverage Δ
utils.go 18.18% <0%> (+18.18%) :arrow_up:
runc.go 9.28% <19.04%> (+3.71%) :arrow_up:
monitor.go 65% <0%> (+65%) :arrow_up:
command_linux.go 70% <0%> (+70%) :arrow_up:

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 a2952bc...469fa2c. Read the comment docs.

crosbymichael commented 4 years ago

LGTM

crosbymichael commented 4 years ago

@sipsma thanks for the fix!