send-file: stream the body instead of buffering the whole file on both sides - #97
Merged
Merged
Conversation
myobie
force-pushed
the
fix/send-file-streams-instead-of-buffering
branch
from
August 29, 2026 13:24
65b2db1 to
1e89b0a
Compare
…h sides Finding 7 of the 2026-08-29 review. The sender read the file whole with std::fs::read and the receiver allocated header.len bytes up front, so a 1.5 GiB transfer cost about 1.5 GiB of resident memory on each daemon at once. On a host with a memory ceiling that is enough to kill the daemon mid-transfer. send_file opens the file and send_from_reader streams exactly len bytes with tokio::io::copy over a bounded buffer; the daemon passes the path rather than reading it. The receiver streams the body straight to its temp file with the same bounded copy, reading exactly header.len bytes via take() so it never waits for an EOF the sender does not send before its ack. Neither side allocates against the file size. The wire format is unchanged, so a streaming build and an old build interoperate. A transfer that ends short of its declared length is now refused and leaves no file, where before a short read would error on read_exact. Tests: a_large_file_streams_through_in_chunks round-trips 5 MiB over a 64 KiB duplex; a_short_stream_is_refused_and_leaves_no_file pins the truncation guard.
myobie
force-pushed
the
fix/send-file-streams-instead-of-buffering
branch
from
August 30, 2026 12:02
1e89b0a to
c42615b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Finding 7 of the 2026-08-29 review (reproduced: receiver peaked +1.0 GB, sender +1.0 GB on a 1 GiB transfer).
The hole
The sender read the file whole (
std::fs::readin the daemon) and passed the slice tosend; the receiver allocatedvec![0u8; header.len]andread_exactinto it. So a 1.5 GiB transfer held about 1.5 GiB of resident memory on each daemon at once. On a host with aMemoryMax, that is enough to kill the daemon mid-transfer, taking every shell, exec and sync session with it.The fix
send_file(stream, name, path)opens the file, reads its size from metadata, andsend_from_readerstreams exactlylenbytes withtokio::io::copyover a bounded buffer. The daemon passes the path instead ofstd::fs::read-ing it.header.lenbytes viatake(len)so it never waits for an EOF the sender does not send before its ack.Neither side allocates against the file size. The wire format is unchanged (header +
lenbody bytes + ack), so a streaming build and an old build interoperate in either direction.Tests
a_large_file_streams_through_in_chunks: round-trips 5 MiB over a 64 KiB duplex (many chunks), bytes intact.a_short_stream_is_refused_and_leaves_no_file: a sender that promises 1000 bytes and sends 10 is refused, and no file (partial or final) reaches the inbox. This is a strict improvement — a short transfer previously errored onread_exact, now it is refused explicitly and cleans up.The memory improvement is structural (no
vec![0; len]on either side); the file that lands is byte-identical, so the unit tests prove correctness and the reviewer's two-daemon RSS measurement is the memory proof.cargo testgreen.