Skip to content

Commit

Permalink
pkg/machine/compression: skip decompress bar for empty file
Browse files Browse the repository at this point in the history
When the file is empty it is possible our code panics as bar.ProxyReader
returns nil when the bar is finished which is the case for 0 size as it
doesn't have to read anything from there. However as this happens on
different goroutines it is race and most of the time still works.

To fix this simply skip the progress bar setup for empty files.

While at it fix the deprecated argument in the tests.

Fixes #23281

Signed-off-by: Paul Holzinger <[email protected]>
  • Loading branch information
Luap99 committed Jul 18, 2024
1 parent 599967b commit f630eeb
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 14 deletions.
35 changes: 22 additions & 13 deletions pkg/machine/compression/decompress.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,27 @@ func runDecompression(d decompressor, decompressedFilePath string) (retErr error
}
defer d.close()

initMsg := progressBarPrefix + ": " + filepath.Base(decompressedFilePath)
finalMsg := initMsg + ": done"

p, bar := utils.ProgressBar(initMsg, d.compressedFileSize(), finalMsg)
// Wait for bars to complete and then shut down the bars container
defer p.Wait()

compressedFileReaderProxy := bar.ProxyReader(compressedFileReader)
// Interrupts the bar goroutine. It's important that
// bar.Abort(false) is called before p.Wait(), otherwise
// can hang.
defer bar.Abort(false)
filesize := d.compressedFileSize()
// bar.ProxyReader() returns nil when the bar is finished.
// When the size is set to 0 the bar will finfish immediately, but this happens
// in a different goroutine so it is race and only happens sometimes.
// In general if the input is an empty file then we do not have to display a
// progress bar at all as there is nothing to copy/extract really
// https://github.com/containers/podman/issues/23281
if filesize > 0 {
initMsg := progressBarPrefix + ": " + filepath.Base(decompressedFilePath)
finalMsg := initMsg + ": done"

p, bar := utils.ProgressBar(initMsg, filesize, finalMsg)
// Wait for bars to complete and then shut down the bars container
defer p.Wait()

compressedFileReader = bar.ProxyReader(compressedFileReader)
// Interrupts the bar goroutine. It's important that
// bar.Abort(false) is called before p.Wait(), otherwise
// can hang.
defer bar.Abort(false)
}

var decompressedFileWriter *os.File

Expand All @@ -94,7 +103,7 @@ func runDecompression(d decompressor, decompressedFilePath string) (retErr error
}
}()

if err = d.decompress(decompressedFileWriter, compressedFileReaderProxy); err != nil {
if err = d.decompress(decompressedFileWriter, compressedFileReader); err != nil {
logrus.Errorf("Error extracting compressed file: %q", err)
return err
}
Expand Down
2 changes: 1 addition & 1 deletion test/system/610-format.bats
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ function check_subcommand() {
# ...or machine. But podman machine is ultra-finicky, it fails as root
# or if qemu is missing. Instead of checking for all the possible ways
# to skip it, just try running init. If it works, we can test it.
run_podman '?' machine init --image-path=/dev/null mymachine
run_podman '?' machine init --image=/dev/null mymachine
if [[ $status -eq 0 ]]; then
can_run_podman_machine=true
extra_args_table+="
Expand Down

0 comments on commit f630eeb

Please sign in to comment.