Skip to content

Commit 6fa0780

Browse files
committed
Perf: Use smb2::FileWriter::abort() on cancelled uploads
`SmbVolume::import_single_file_with_progress` and `write_from_stream` called `writer.finish()` on cancel, which forces a server-side FLUSH (fsync) on a file we're about to delete anyway. Swap to the new `writer.abort()` from smb2 `1d91384`, which drains in-flight WRITE responses to keep credits/message IDs in sync but skips the fsync and does a best-effort CLOSE. Saves ~100 ms to ~1 s on slow NAS cancels. Also bumped Cargo.lock to pick up smb2 `1d91384c` (the `abort()` release).
1 parent d56c1df commit 6fa0780

3 files changed

Lines changed: 32 additions & 31 deletions

File tree

Cargo.lock

Lines changed: 14 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

apps/desktop/src-tauri/src/file_system/volume/smb.rs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -374,11 +374,12 @@ impl SmbVolume {
374374
bytes_done += n as u64;
375375

376376
if on_progress(bytes_done, total_size) == std::ops::ControlFlow::Break(()) {
377-
// Finalize the writer to drain in-flight WRITE responses and
378-
// close the handle — if we just drop it, stale responses
379-
// poison the session for the next op. Best-effort delete of
380-
// the partial file afterwards.
381-
let _ = writer.finish().await;
377+
// Abort drains in-flight WRITE responses and closes the handle
378+
// without the server-side fsync that `finish()` would force
379+
// (we're about to delete the partial file anyway). Dropping
380+
// the writer directly would leave stale responses on the
381+
// connection and poison the next op.
382+
let _ = writer.abort().await;
382383
let _ = client.delete_file(tree, smb_path).await;
383384
return Err(VolumeError::Cancelled("Operation cancelled by user".to_string()));
384385
}
@@ -1385,11 +1386,12 @@ impl Volume for SmbVolume {
13851386
bytes_read += chunk.len() as u64;
13861387

13871388
if on_progress(bytes_read, size) == std::ops::ControlFlow::Break(()) {
1388-
// Finalize the writer to drain in-flight WRITE responses
1389-
// and close the handle — dropping it directly would leave
1390-
// stale responses on the connection, poisoning the next op.
1391-
// Best-effort delete of the partial file afterwards.
1392-
let _ = writer.finish().await;
1389+
// Abort drains in-flight WRITE responses and closes the
1390+
// handle without the server-side fsync that `finish()`
1391+
// would force (we're about to delete the partial file
1392+
// anyway). Dropping directly would leave stale responses
1393+
// on the connection and poison the next op.
1394+
let _ = writer.abort().await;
13931395
let _ = client.delete_file(tree, &smb_path).await;
13941396
return Err(VolumeError::Cancelled("Operation cancelled by user".to_string()));
13951397
}
@@ -2626,7 +2628,7 @@ mod tests {
26262628
async fn smb_integration_import_from_local_cancel_mid_write() {
26272629
// Cancel partway through a multi-chunk import via progress-break.
26282630
// Verifies Cancelled is returned and that the SMB session is still
2629-
// usable for subsequent ops (writer.finish() drains in-flight WRITE
2631+
// usable for subsequent ops (writer.abort() drains in-flight WRITE
26302632
// responses cleanly on cancel, best-effort-deletes the partial file).
26312633
let vol = make_docker_volume().await;
26322634
let dir = test_dir_name();

apps/desktop/src/lib/file-operations/CLAUDE.md

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,8 @@ When directory has parent entry shown at index 0, frontend indices are offset by
126126
- **Scan preview reuse**: TransferDialog starts a scan preview on mount. If the user confirms before the scan finishes,
127127
the scan keeps running (TransferDialog sets `confirmed = true` and skips cancellation in `onDestroy`).
128128
TransferProgressDialog picks up listening to the same scan events via `scanInProgress` prop. `waitForScanThenStart`
129-
subscribes to the scan events first, then awaits `checkScanPreviewStatus()`. Both the `scan-preview-complete`
130-
listener AND the status check can signal "ready to start" — especially for fast scans that complete during the
131-
status-check `await`. Both paths converge on a local `kickOff()` helper guarded by a `started` flag, so
132-
`startOperation()` dispatches exactly once. The scan-error and scan-cancelled listeners also flip `started = true`
133-
as a terminal signal, so a late `scan-preview-complete` event can't dispatch an operation after we've errored or
134-
cancelled.
129+
subscribes to the scan events first, then awaits `checkScanPreviewStatus()`. Both the `scan-preview-complete` listener
130+
AND the status check can signal "ready to start" — especially for fast scans that complete during the status-check
131+
`await`. Both paths converge on a local `kickOff()` helper guarded by a `started` flag, so `startOperation()`
132+
dispatches exactly once. The scan-error and scan-cancelled listeners also flip `started = true` as a terminal signal,
133+
so a late `scan-preview-complete` event can't dispatch an operation after we've errored or cancelled.

0 commit comments

Comments
 (0)