Skip to content

Eliminate deep recursion risk in cross-disk directory copy (cross_copy_single) #3

Description

@koraytaylan

Eliminate Deep Recursion Risk in Cross-Disk Directory Copy (cross_copy_single)

Summary

The function that copies (or moves) files and directories between any two storage backends (cross_copy_single) is implemented using unbounded recursion.

// backend/src/commands/file.rs:210
Box::pin(cross_copy_single(src, dst, &child.path, &child_dst)).await?;

For every directory level in the tree being copied, the call stack grows by one frame. While this works for normal user data (10–20 levels deep), it will crash with a stack overflow on extremely deep directory structures (common in some backup datasets, generated code trees, or malicious inputs).

What exactly is the problem?

Current implementation (backend/src/commands/file.rs, around lines 197–217):

async fn cross_copy_single(...) -> Result<(), DiskDeckError> {
    let stat = src.stat(src_path).await?;
    if stat.is_dir {
        dst.create_dir(dst_path).await?;
        let children = src.list(src_path).await?;
        for child in children {
            let child_dst = ...;
            Box::pin(cross_copy_single(src, dst, &child.path, &child_dst)).await?;  // <-- recursion
        }
    } else {
        let data = src.read(src_path).await?;
        dst.write(dst_path, &data).await?;
    }
    Ok(())
}

This is called from:

  • run_cross_copy_loop
  • run_cross_move_loop

The recursion depth = the maximum depth of any directory tree being copied across backends.

Why this is a real bug (not theoretical)

  • The Rust default stack size per thread/task is usually 2 MiB or 8 MiB.
  • Each async frame + the Box::pin overhead still consumes stack until the first .await inside the recursive call.
  • Some users legitimately have directory trees 2,000–10,000 levels deep (especially on network filesystems or tarballs from scientific computing).
  • A single deep tree copy or move operation can panic the entire backend and crash the Tauri app for the user.

We already have excellent tests for this function (cross_copy_single_directory, run_cross_copy_loop_*, etc.). We just never stress-tested extreme depth.

Why the current Box::pin does NOT fully solve it

Box::pin moves the future onto the heap, which is good for the size of the future. However, the synchronous call stack still grows while we execute the code before we hit an .await point inside the recursive call. The list() and create_dir() are awaited, but the recursive Box::pin(...).await itself is still a regular function call that grows the stack.

How to fix it (junior-dev friendly approach)

Recommended Solution: Convert to an Iterative Work-Queue (Breadth-First or Depth-First with explicit stack)

This is the safest, most idiomatic, and easiest-to-understand fix for async Rust.

High-level design

Instead of the function calling itself, we maintain an explicit VecDeque (or Vec) of "work items" that still need to be processed:

struct CopyWork {
    src_path: String,
    dst_path: String,
}

The main loop repeatedly takes one item from the queue, does the work, and pushes new child items onto the queue when it discovers a directory.

This completely eliminates Rust call-stack growth.

Detailed Implementation Plan

  1. Read and understand the existing code (very important)

    • Read the full cross_copy_single function (lines ~197-217)
    • Read run_cross_copy_loop (lines ~413-440)
    • Read run_cross_move_loop (lines ~443-472)
    • Read the tests at the bottom of the file (they will still pass after your change)
  2. Create a new internal function (or replace the recursive one) called something like:

    pub(crate) async fn copy_tree(
        src: &Arc<dyn StorageBackend>,
        dst: &Arc<dyn StorageBackend>,
        src_root: &str,
        dst_root: &str,
    ) -> Result<(), DiskDeckError>
  3. Implement it iteratively

    Suggested skeleton (you will fill in the details):

    use std::collections::VecDeque;
    
    let mut pending: VecDeque<(String, String)> = VecDeque::new();
    pending.push_back((src_root.to_string(), dst_root.to_string()));
    
    while let Some((src_path, dst_path)) = pending.pop_front() {
        // 1. stat the src_path
        // 2. if directory:
        //      - create dst directory
        //      - list children
        //      - for each child, push (child_src, child_dst) into pending
        // 3. else (file):
        //      - read + write
        // 4. (Important) check cancel_flag periodically if you have access to it
    }
  4. Handle progress reporting

    The current loops in run_cross_copy_loop report progress only for the top-level selected items. Your new iterative version should keep the same behavior for now (or you can improve it as a stretch goal — see "Out of Scope").

  5. Update the callers

    • cross_copy_entries and cross_move_entries call the run_*_loop functions, which currently call the recursive helper.
    • You will change the inner implementation, not the public Tauri command signatures.
  6. Keep all existing tests green

    There are many good tests:

    • cross_copy_single_file
    • cross_copy_single_directory
    • run_cross_copy_loop_copies_between_backends
    • cancellation tests, etc.

    Your change must not break any of them.

Acceptance Criteria

  • The recursive cross_copy_single function is removed or no longer used for the main copy path
  • Directory copying (including deeply nested trees) now uses an explicit work queue (VecDeque or similar)
  • All existing tests in commands/file.rs still pass (cargo test --test file or full cargo test)
  • cargo clippy -- -D warnings passes with no new warnings
  • You add a regression test that creates a directory tree at least 50 levels deep and successfully copies it between two MemoryBackend instances (the in-memory test backend)
  • The PR description explains in 2-3 sentences why recursion was risky and how the iterative approach removes the risk
  • The function still correctly handles cancellation (check the AtomicBool between items)

Learning Goals (this issue will teach you)

  • The difference between recursive and iterative algorithms in practice
  • How to use VecDeque as an explicit stack/queue in Rust
  • Why async recursion is subtle even when using Box::pin
  • How to write a test that would have caught the original bug (deep tree)
  • Safe refactoring of security-sensitive file operation code

Files You Will Touch

  • backend/src/commands/file.rs (main work)
  • Possibly small updates in backend/src/commands/mod.rs if you export a new helper

Out of Scope for This First Iteration (good follow-up issues)

  • Reporting progress per file inside a directory (currently only top-level items report progress)
  • Parallelizing the copy (we can do multiple files concurrently later)
  • Adding a configurable maximum depth safety limit with a clear error

Difficulty: Medium (great learning issue for a junior who is comfortable with async Rust and wants to level up their algorithms + testing skills).

Mentorship note: This is a safe change because we have excellent test coverage on exactly this code path. A junior dev who completes this issue will have removed a real (if rare) crash vector and will understand iterative tree processing deeply.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggood first issueGood for newcomershelp wantedExtra attention is needed

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions