From f630eebcfac11887cc9e6f5b10360c1436407442 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 18 Jul 2024 13:17:49 +0200 Subject: [PATCH] pkg/machine/compression: skip decompress bar for empty file 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 --- pkg/machine/compression/decompress.go | 35 +++++++++++++++++---------- test/system/610-format.bats | 2 +- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/pkg/machine/compression/decompress.go b/pkg/machine/compression/decompress.go index c13335307b7a..5e2448f5db21 100644 --- a/pkg/machine/compression/decompress.go +++ b/pkg/machine/compression/decompress.go @@ -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 @@ -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 } diff --git a/test/system/610-format.bats b/test/system/610-format.bats index 448d1e6ed626..165c50127d15 100644 --- a/test/system/610-format.bats +++ b/test/system/610-format.bats @@ -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+="