Skip to content

Commit 709a3f8

Browse files
authored
Merge pull request #1228 from jeckersb/broken-pipe
ostree-ext/filter_tar_async: flush decompressor stream
2 parents af9d4a9 + c3e0ba6 commit 709a3f8

File tree

1 file changed

+21
-11
lines changed

1 file changed

+21
-11
lines changed

ostree-ext/src/tar/write.rs

Lines changed: 21 additions & 11 deletions
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)