Repository navigation
*: io.Copy improvements - #437
Conversation
d90b401 to
f9700be
Compare
This is necessary to make sure we don't error out for no reason. In addition, there have been cases where errors are not correctly passed on through CloseWithError, which could result in truncated data being written into images. Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
While the Go stdlib bug has been fixed[1], closing a single pipe multiple times with different CloseWithError() calls doesn't make much sense (even if they are now a no-op). In addition, log a warning whenever we use CloseWithError() because we've observed flaky tests and bug reports where even though an error should've been propogated all the way up through CloseWithError() it seems that the error still got lost. This way at least you can tell an issue occurred during generation. [1]: https://golang.org/issue/24283 Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
f9700be to
2163223
Compare
Codecov Report
@@ Coverage Diff @@
## main #437 +/- ##
==========================================
- Coverage 74.12% 73.78% -0.34%
==========================================
Files 59 60 +1
Lines 4831 4872 +41
==========================================
+ Hits 3581 3595 +14
- Misses 898 923 +25
- Partials 352 354 +2
|
|
/cc @mikemccracken |
|
I'm a little suspicious that we're hitting EINTR; most of these files we're writing to will have been created via the dir cas backend's PutBlob(), which it looks like uses ioutil.TempFile(), which doesn't open things as O_NONBLOCK AFAIK. The patch itself looks fine to me, though. |
|
@tych0 From a few Go releases ago, you can get |
|
Yeah, I've heard about this change. But man, that sucks. Really feels like you should not have to deal with EINTR from every possible function call that could cause a kernel sleep... |
|
Yeah I don't understand why they didn't handle this in the stdlib -- the behaviour of |
io.Copyusers to avoid erroring out for no reason.These changes might help with #436 but I'm not sure whether these were the root cause of the issue.
Signed-off-by: Aleksa Sarai cyphar@cyphar.com