Skip to content

Commit 63b6728

Browse files
committed
Volume copy/move: drop unsafe transmute, take Arc<dyn OperationEventSink>
Widens the three migrated inner functions (`copy_volumes_with_progress`, `move_volumes_with_progress`, `move_within_same_volume_with_progress`) to take `Arc<dyn OperationEventSink>` instead of `&dyn OperationEventSink`. Each closure passed to `drive_transfer_serial_async` now `Arc::clone(&events)`s into its environment alongside the other Arc'd captures — uniform shape, no special-cases, no lifetime gymnastics. - Removes three `unsafe { std::mem::transmute::<&dyn OperationEventSink, &'static dyn OperationEventSink>(events) }` blocks. Zero `unsafe` and zero `transmute` left in `write_operations/volume_copy.rs` and `write_operations/volume_move.rs`. - Production callers (`copy_between_volumes`, `move_between_volumes`, `move_within_same_volume`) wrap `TauriEventSink::new(app)` in an `Arc<dyn OperationEventSink>` and pass `Arc::clone(&events)`. Public Tauri command signatures unchanged. - Test call sites mechanical: `events.as_ref()` becomes `events.clone()` (works because of `CoerceUnsized` at the function-arg boundary). `Arc<CollectorEventSink>` keeps field access for assertions; the call site coerces to `Arc<dyn OperationEventSink>` on the way in. - Two SMB integration test call sites (`smb_integration_copy_100_unique_files`, soak loop) updated to construct `Arc::new(CollectorEventSink::new())`. - All 46 unit tests in `volume_copy_tests.rs` + `volume_move_tests.rs` and the full `./scripts/check.sh --rust` suite (1813 unit + 28 integration) pass without semantic changes to any test body. - Net diff vs the previous transmute-based migration: -215 lines (mostly the now-removed SAFETY justification comments and the per-function `events_static` setup).
1 parent 0218a64 commit 63b6728

5 files changed

Lines changed: 114 additions & 108 deletions

File tree

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3562,12 +3562,12 @@ mod tests {
35623562
));
35633563

35643564
let state = Arc::new(WriteOperationState::new(Duration::from_millis(200)));
3565-
let events = CollectorEventSink::new();
3565+
let events = Arc::new(CollectorEventSink::new());
35663566
let config = VolumeCopyConfig::default();
35673567

35683568
let copy_start = Instant::now();
35693569
let result = copy_volumes_with_progress(
3570-
&events,
3570+
events.clone(),
35713571
"test-op-100-unique",
35723572
&state,
35733573
Arc::clone(&vol),
@@ -3882,12 +3882,12 @@ mod tests {
38823882
local_dir.path().to_path_buf(),
38833883
));
38843884
let state = Arc::new(WriteOperationState::new(Duration::from_millis(200)));
3885-
let events = CollectorEventSink::new();
3885+
let events = Arc::new(CollectorEventSink::new());
38863886
let config = VolumeCopyConfig::default();
38873887

38883888
let iter_start = Instant::now();
38893889
let result = copy_volumes_with_progress(
3890-
&events,
3890+
events.clone(),
38913891
&format!("soak-iter-{iter_idx}"),
38923892
&state,
38933893
Arc::clone(&vol),

apps/desktop/src-tauri/src/file_system/write_operations/volume_copy.rs

Lines changed: 33 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,9 @@ pub async fn copy_between_volumes(
164164
let operation_id_for_cleanup = operation_id_for_spawn.clone();
165165
let app_for_error = app.clone();
166166

167-
let events = TauriEventSink::new(app);
167+
let events: Arc<dyn OperationEventSink> = Arc::new(TauriEventSink::new(app));
168168
let result: Result<(), WriteFailure> = copy_volumes_with_progress(
169-
&events,
169+
Arc::clone(&events),
170170
&operation_id_for_spawn,
171171
&state,
172172
source_volume,
@@ -360,12 +360,20 @@ fn account_skipped_file(
360360
/// `volume/smb.rs`) can drive the real copy pipeline with a
361361
/// `CollectorEventSink` instead of spinning up a full Tauri app. In
362362
/// production, the only caller is `copy_between_volumes` in this file.
363+
///
364+
/// Takes `Arc<dyn OperationEventSink>` (not `&dyn`) because closures passed
365+
/// to `drive_transfer_serial_async` are bounded
366+
/// `for<'a> FnMut(...) -> Pin<Box<dyn Future + Send + 'a>>` — their returned
367+
/// futures must be valid for any input lifetime including `'static`, so the
368+
/// closures can't borrow outer-fn `&` args. `Arc::clone(&events)` into each
369+
/// closure is the clean way out; the caller and tests already wrap the sink
370+
/// in an Arc so the boundary is a no-op.
363371
#[allow(
364372
clippy::too_many_arguments,
365373
reason = "Volume copy requires passing multiple context parameters"
366374
)]
367375
pub(crate) async fn copy_volumes_with_progress(
368-
events: &dyn OperationEventSink,
376+
events: Arc<dyn OperationEventSink>,
369377
operation_id: &str,
370378
state: &Arc<WriteOperationState>,
371379
source_volume: Arc<dyn Volume>,
@@ -428,7 +436,7 @@ pub(crate) async fn copy_volumes_with_progress(
428436
);
429437

430438
state.emit_progress_via_sink(
431-
events,
439+
&*events,
432440
WriteProgressEvent::new(
433441
operation_id.to_string(),
434442
WriteOperationType::Copy,
@@ -535,7 +543,7 @@ pub(crate) async fn copy_volumes_with_progress(
535543

536544
// Emit initial copying phase event
537545
state.emit_progress_via_sink(
538-
events,
546+
&*events,
539547
WriteProgressEvent::new(
540548
operation_id.to_string(),
541549
WriteOperationType::Copy,
@@ -590,7 +598,7 @@ pub(crate) async fn copy_volumes_with_progress(
590598
bulk_skip_bytes
591599
);
592600
state.emit_progress_via_sink(
593-
events,
601+
&*events,
594602
WriteProgressEvent::new(
595603
operation_id.to_string(),
596604
WriteOperationType::Copy,
@@ -685,7 +693,7 @@ pub(crate) async fn copy_volumes_with_progress(
685693
&dest_volume,
686694
&dest_item_path,
687695
config,
688-
events,
696+
&*events,
689697
operation_id,
690698
state,
691699
&mut apply_to_all_resolution,
@@ -708,7 +716,7 @@ pub(crate) async fn copy_volumes_with_progress(
708716
&last_progress_mutex,
709717
progress_interval,
710718
state,
711-
events,
719+
&*events,
712720
operation_id,
713721
total_files,
714722
total_bytes,
@@ -732,7 +740,7 @@ pub(crate) async fn copy_volumes_with_progress(
732740
let src_vol = Arc::clone(&source_volume);
733741
let dst_vol = Arc::clone(&dest_volume);
734742
let state_clone = Arc::clone(state);
735-
let events_ref: &dyn OperationEventSink = events;
743+
let events_task = Arc::clone(&events);
736744
let op_id = operation_id;
737745
let files_done_a = Arc::clone(&files_done_atomic);
738746
let bytes_done_a = Arc::clone(&atomic_bytes_done);
@@ -761,7 +769,7 @@ pub(crate) async fn copy_volumes_with_progress(
761769
if last.elapsed() >= progress_interval {
762770
*last_prog_a.lock().unwrap() = Instant::now();
763771
state_clone.emit_progress_via_sink(
764-
events_ref,
772+
&*events_task,
765773
WriteProgressEvent::new(
766774
op_id.to_string(),
767775
WriteOperationType::Copy,
@@ -878,29 +886,14 @@ pub(crate) async fn copy_volumes_with_progress(
878886
};
879887
// The driver bounds its closures as
880888
// `for<'a> FnMut(...) -> Pin<Box<dyn Future + Send + 'a>>` — the
881-
// returned future must be valid for **any** input lifetime `'a`,
882-
// including `'static`. Closures that capture outer-fn `&` args end
883-
// up with futures bounded by those args' lifetimes, which the
884-
// for-all bound rejects ("returning this value requires that `'N`
885-
// must outlive `'static`"). The captures are sound in practice
886-
// (the driver awaits inline and never holds closures past this
887-
// function's end), but we have to convince the compiler.
888-
//
889-
// Strategy: clone what's cheap to clone, lifetime-extend the rest
890-
// via `transmute` scoped to this block. `config` clones to owned
891-
// (Clone), `operation_id` to `String`. `events` is a trait object
892-
// whose concrete impl isn't clonable in general; transmute its
893-
// borrow to `'static` and capture that.
889+
// returned future must be valid for any input lifetime `'a`,
890+
// including `'static`. Outer-fn `&` arg captures yield futures
891+
// bounded by those args' lifetimes, which the for-all bound
892+
// rejects. `config` and `operation_id` clone cheaply; `events` is
893+
// already an `Arc<dyn OperationEventSink>` on entry, so each
894+
// closure `Arc::clone(&events)`s into its environment.
894895
let config_owned: VolumeCopyConfig = config.clone();
895896
let operation_id_owned: String = operation_id.to_string();
896-
// SAFETY: `events` outlives every closure invocation made by the
897-
// driver: the driver awaits in-place at `.await` below, so the
898-
// closures stop being called before this function returns. None of
899-
// the closures or the futures they produce escape this scope; the
900-
// `'static`-erased reference is dropped before the original
901-
// `events` borrow ends.
902-
let events_static: &'static dyn OperationEventSink =
903-
unsafe { std::mem::transmute::<&dyn OperationEventSink, &'static dyn OperationEventSink>(events) };
904897
// Per-source mutable state shared with the driver's closures via
905898
// interior mutability. Avoids `&mut` captures (which would force
906899
// `AsyncFnMut` semantics; the driver bounds the closures as plain
@@ -914,7 +907,7 @@ pub(crate) async fn copy_volumes_with_progress(
914907
let source_hints_arc: Arc<HashMap<PathBuf, SourceHint>> = Arc::new(std::mem::take(&mut source_hints));
915908

916909
let outcome = drive_transfer_serial_async(
917-
events,
910+
&*events,
918911
state,
919912
operation_id,
920913
source_paths,
@@ -945,6 +938,7 @@ pub(crate) async fn copy_volumes_with_progress(
945938
let source_volume = Arc::clone(&source_volume);
946939
let dest_volume = Arc::clone(&dest_volume);
947940
let state = Arc::clone(state);
941+
let events = Arc::clone(&events);
948942
let apply_to_all = Arc::clone(&apply_to_all_cell);
949943
let source_hints = Arc::clone(&source_hints_arc);
950944
let config = config_owned.clone();
@@ -953,6 +947,7 @@ pub(crate) async fn copy_volumes_with_progress(
953947
let source_volume = Arc::clone(&source_volume);
954948
let dest_volume = Arc::clone(&dest_volume);
955949
let state = Arc::clone(&state);
950+
let events = Arc::clone(&events);
956951
let apply_to_all = Arc::clone(&apply_to_all);
957952
let source_hints = Arc::clone(&source_hints);
958953
let config = config.clone();
@@ -985,7 +980,7 @@ pub(crate) async fn copy_volumes_with_progress(
985980
&dest_volume,
986981
&initial_dest_owned,
987982
&config,
988-
events_static,
983+
&*events,
989984
&operation_id,
990985
&state,
991986
&mut latched,
@@ -1012,6 +1007,7 @@ pub(crate) async fn copy_volumes_with_progress(
10121007
let source_volume = Arc::clone(&source_volume);
10131008
let dest_volume = Arc::clone(&dest_volume);
10141009
let state = Arc::clone(state);
1010+
let events = Arc::clone(&events);
10151011
let last_dest_cell = Arc::clone(&last_dest_cell);
10161012
let failure_ctx_cell = Arc::clone(&failure_ctx_cell);
10171013
let copied_paths = Arc::clone(&copied_paths_for_closure);
@@ -1021,6 +1017,7 @@ pub(crate) async fn copy_volumes_with_progress(
10211017
let source_volume = Arc::clone(&source_volume);
10221018
let dest_volume = Arc::clone(&dest_volume);
10231019
let state = Arc::clone(&state);
1020+
let events = Arc::clone(&events);
10241021
let last_dest_cell = Arc::clone(&last_dest_cell);
10251022
let failure_ctx_cell = Arc::clone(&failure_ctx_cell);
10261023
let copied_paths = Arc::clone(&copied_paths);
@@ -1052,6 +1049,7 @@ pub(crate) async fn copy_volumes_with_progress(
10521049
let last_emit = std::sync::Mutex::new(Instant::now());
10531050
let file_name_for_cb = file_name.clone();
10541051
let state_for_cb = Arc::clone(&state);
1052+
let events_for_cb = Arc::clone(&events);
10551053
let on_file_progress = |file_bytes_done: u64, _file_bytes_total: u64| -> ControlFlow<()> {
10561054
if is_cancelled(&state_for_cb.intent) {
10571055
return ControlFlow::Break(());
@@ -1062,7 +1060,7 @@ pub(crate) async fn copy_volumes_with_progress(
10621060
drop(last);
10631061
let current_total = bytes_done_so_far + file_bytes_done;
10641062
state_for_cb.emit_progress_via_sink(
1065-
events_static,
1063+
&*events_for_cb,
10661064
WriteProgressEvent::new(
10671065
operation_id.clone(),
10681066
WriteOperationType::Copy,
@@ -1217,7 +1215,7 @@ pub(crate) async fn copy_volumes_with_progress(
12171215
let rollback_completed = volume_rollback_with_progress(
12181216
&dest_volume,
12191217
&copied_paths,
1220-
events,
1218+
&*events,
12211219
operation_id,
12221220
state,
12231221
files_done,

0 commit comments

Comments
 (0)