Repository navigation
fix(sftp): a tar stream stranded on a dead link resumes per file (#495) - #560
Merged
Merged
Conversation
Per-file copies run under a stall watchdog; tar streams (upload, download, relay) had none, so with SSH keepalive off a half-open link hung a folder transfer forever. after_tar now runs the stream under unless_lost, fed by the bytes the stream counts into its Progress. A stalled stream is kept while the link comes back, and cut (by cancelling its own token, so it closes its remote end as a cancel does) only once it is stranded: its connection closed, or a reconnect replaced it and the old one no longer answers. A cut stream resumes per file. A link that answers again keeps the stream, and the watchdog never fails a transfer by itself (#554). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kipavy
force-pushed
the
fix/tar-stall-watchdog-495
branch
from
October 7, 2026 10:45
7df5b4e to
4a9d641
Compare
Contributor
Author
|
Live tests run against a real SSH container (
Check that the "reconnect while alive" test can fail: with the looser rule (replaced ⇒ stranded) it fails, because the per-file fallback ran on a live stream. So it guards the tightening. |
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.
Closes #495. Closes #554.
Per-file copies already run under a stall watchdog (
unless_lost). Tar streams (stream::upload/download/relay) did not. With SSH keepalive off, a half-open link hung a folder transfer forever instead of falling back to per-file resume.What changed
after_tarnow starts the stream itself and runs it under the watchdog. The watchdog's progress counter is the byte count the stream already keeps in itsProgress. Both tar entry points (stream_or,relay_or_per_file) go through it.After a stall of 15 s or more, the stream is kept while
revivewaits for the link (the UI shows "waiting"). It is only cut once it is stranded:ssh_answers).A cut stream is stopped by cancelling its own child token, not by dropping it. It runs the same bounded unwind as a user cancel: it closes the remote channel, stops the status task and the local packer/unpacker. It gets
UNWIND(5 s) for that before the per-file run starts. The watchdog never fails a transfer on its own.Plumbing:
unless_losttakes aStuckcheck (a smallasync_trait, so the future staysSend) that counts as dead at a stall. Per-file copies pass&false, so they are unchanged.StartedOnrecords eachTarHost's SSH handle when the stream starts and implementsStuck._viafunctions take the cut token and theProgress.Job::with_progressbuilds their job, andJob::newis now test-only.resume_per_fileis shared by both fallback paths.The one case left
A connection that stops answering, is replaced by a reconnect, and then comes back to life later could deliver its last buffered bytes to the remote
tarafter the per-file run has started. Before this PR that transfer hung, and finished only if the old connection revived. A reconnect normally happens because the old flow is gone for good (Wi-Fi roam, NAT reset). Ruling this out completely would mean killing the remotetarby PID over the new connection, which changes the remote command on every host type. This PR doesn't do that.Tests
New unit tests (paused clock; tokio
test-utiladded to dev-deps only):a_tar_stream_stranded_on_a_dead_link_resumes_per_file_instead_of_hanginga_tar_stream_left_on_a_replaced_connection_resumes_per_filea_cut_tar_stream_runs_its_own_cancel_before_the_fallbacka_stalled_tar_stream_whose_own_connection_answers_again_is_kept: a probe fails once, the stream finishes by itself, it is never cut, and there is no per-file run.Each was seen failing first. The "kept" test fails with cut-on-any-probe logic. The "replaced connection" test hangs without the
Stuckcheck.New live tests (
#[ignore = "needs docker"]). They throttle the SSH container (--cpus 0.25) andSIGSTOPpart of a running upload:a_tar_upload_whose_link_freezes_and_thaws_finishes_on_its_own_stream: the tar'ssshdis stopped for 25 s, thenSIGCONT. The upload finishes on its stream, "waiting" was seen, there is no per-file run, and the tree is identical.a_tar_upload_stranded_by_a_reconnect_resumes_per_file_on_the_new_link: the tar'ssshdis stopped and a fresh connection is swapped into the session. The upload goes per file and the tree is identical.a_tar_upload_whose_session_reconnects_while_its_link_lives_finishes_on_it: the remotetaritself is stopped (its link still answers), a fresh connection is swapped in, and after 30 s it getsSIGCONT. The upload finishes on its stream with no per-file run.The live tests compile but have not been run. The build container here has no Docker access. To run them:
cargo test --lib commands::sftp::tar::tests::live -- --ignored --test-threads=1Verification:
cargo fmt --check: clean.cargo clippy --workspace --all-targets -D warnings: clean.cargo test --lib: 785 passed, 3 failed. The 3 failures are thecommands::pluginstests that needpnpm build:pluginsoutput, the same as on dev.🤖 Generated with Claude Code