[podman-5.8] storage: wait for tar-split completion to avoid reader race#984
[podman-5.8] storage: wait for tar-split completion to avoid reader race#984haircommander wants to merge 1 commit into
Conversation
Switch to `NewInputTarStreamWithDone` and explicitly wait for the tar-split goroutine to finish before returning. This prevents races with `uncompressed.Close()` while tar-split is still reading. Fixes: podman-container-tools#148 Signed-off-by: Jan Kaluza <jkaluza@redhat.com>
github-actions
Bot
added
storage
|
(note, I really want it in 5.7 branch, but I figure it should be pulled back all the way? if it doesn't need to be, I'd close this in favor of 5.7 directly...) |
|
podman-5.7 is dead, we only support the latest versions podman-5.8 is a bit of a special case since we still have to support 5.8 with security fixes at least until f44 goes EOL but even then we likely do not plan extra work for storage,image,common releases unless really needed. |
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
The backport itself looks reasonable.
go.* have a lot of extra changes which seem to be unnecessary for the tar-split update.
| github.com/VividCortex/ewma v1.2.0 // indirect | ||
| github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d // indirect | ||
| github.com/chzyer/readline v1.5.1 // indirect | ||
| github.com/clipperhouse/uax29/v2 v2.7.0 // indirect |
There was a problem hiding this comment.
This has a lot of unrelated package changes.
| github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 // indirect | ||
| github.com/felixge/httpsnoop v1.0.4 // indirect | ||
| github.com/go-jose/go-jose/v4 v4.0.5 // indirect | ||
| github.com/go-jose/go-jose/v4 v4.1.3 // indirect |
At some point, in c/image CRI-O was maintaining its own brackport branches, under CRI-O’s rules and responsibility. That wouldn’t help if the fix also needed to land in the |
For RHEL we decided to not accept any custom upstream branches any more, any older code backports RHEL need to be hosted on another repo location I think it is important as part of the CNCF to give equal right to everyone. If we maintain special branches for cri-o than other consumers can request the same thing. From a repo access POV that is just not a good idea, only the maintainers per our governance should have repo access. And I do not want to review/merge a bunch of backports for somebody else. So access wise having people maintain their own forks seems a lot saner. The alternative is we maintainers actually offer supported LTS branches for all consumers where we accept backports. So far we only do this for podman/buildah/skopeo and there we only support the latest release which is like a 3 month timeframe, podman 5.8 being special as we will support that one for much longer than normal. |
A very fair point. |
4 participants
Switch to
NewInputTarStreamWithDoneand explicitly wait for the tar-split goroutine to finish before returning.This prevents races with
uncompressed.Close()while tar-split is still reading.Fixes: #148
cherry-pick of #801 I did exclude some of the go work updates (new golang/ x pages etc, so I'm not sure this will work