Skip to content

Commit c3e0ba6

Browse files
committed
ostree-ext/filter_tar_async: flush decompressor stream
We need to read anything past the tar stream, otherwise we'll deadlock via skopeo in FinishPipe. Also remove the bit where we hold the pipe open longer, we shouldn't need to do that anymore because we've ensured we've read everything out of it by this point. Resolves: #1204 Signed-off-by: John Eckersberg <[email protected]>
1 parent d9b18c8 commit c3e0ba6

File tree

1 file changed

+21
-11
lines changed

1 file changed

+21
-11
lines changed

ostree-ext/src/tar/write.rs

+21-11
Original file line numberDiff line numberDiff line change
@@ -366,22 +366,32 @@ async fn filter_tar_async(
366366
let dest = tokio_util::io::SyncIoBridge::new(tx_buf);
367367

368368
let r = filter_tar(&mut src, dest, &config, &repo_tmpdir);
369+
370+
// We need to make sure to flush out the decompressor here,
371+
// otherwise it's possible that we finish processing the tar
372+
// stream but leave data in the pipe. For example,
373+
// zstd:chunked layers will have metadata/skippable frames at
374+
// the end of the stream. That data isn't relevant to the tar
375+
// stream, but if we don't read it here then on the skopeo
376+
// proxy we'll block trying to write the end of the stream.
377+
// That in turn will block our client end trying to call
378+
// FinishPipe, and we end up deadlocking ourselves through
379+
// skopeo.
380+
//
381+
// https://github.com/bootc-dev/bootc/issues/1204
382+
let mut sink = std::io::sink();
383+
let n = std::io::copy(&mut src, &mut sink)?;
384+
if n != 0 {
385+
tracing::debug!("Read extra {n} bytes at end of decompressor stream");
386+
}
387+
369388
// Pass ownership of the input stream back to the caller - see below.
370-
Ok((r, src))
389+
Ok(r)
371390
});
372391
let copier = tokio::io::copy(&mut rx_buf, &mut dest);
373392
let (r, v) = tokio::join!(tar_transformer, copier);
374393
let _v: u64 = v?;
375-
let (r, src) = r?;
376-
// Note that the worker thread took temporary ownership of the input stream; we only close
377-
// it at this point, after we're sure we've done all processing of the input. The reason
378-
// for this is that both the skopeo process *or* us could encounter an error (see join_fetch).
379-
// By ensuring we hold the stream open as long as possible, it ensures that we're going to
380-
// see a remote error first, instead of the remote skopeo process seeing us close the pipe
381-
// because we found an error.
382-
drop(src);
383-
// And pass back the result
384-
r
394+
r?
385395
}
386396

387397
/// Write the contents of a tarball as an ostree commit.

0 commit comments

Comments
 (0)