FlutterconcurrencyShellby

A Missing File Is Not a Reservation: Fixing Shellby's SFTP Download Queue

A Shellby development log: concurrent downloads could both pass an existence check and write to the same path. Exclusive creation fixes name allocation, but local write progress, cancellation cleanup, and resume generations must agree too.

Shellby’s Flutter SFTP queue allows two concurrent transfers. Downloads avoid existing names by adding (1), (2), and so on. The UI already appeared to offer protection against overwriting files, and there really was an existence check in the code.

The missing part was ownership. Observing that a path is available does not reserve it for the operation that follows.

Commit 64babba, dated November 3, reworked this download path. This is a development record based on that implementation and its regression cases. It does not establish that the change has shipped in a store release, or that source review amounts to device validation.

Two honest checks, one destination

Previously, the page selected a local filename before handing the path to the transfer manager. Its essential check was:

if (!await file.exists()) return file;
// Otherwise try file (1), file (2), ...

The task opened the file later. A new download used openWrite(mode: FileMode.write), leaving this possible interleaving:

A: report.txt does not exist; select report.txt
B: report.txt does not exist; select report.txt
A: open report.txt and start writing
B: open the same path and start writing

Neither existence check was wrong. Neither operation had created anything yet.

This race does not require two threads. Other work can advance while an asynchronous operation is suspended. The mistake was treating a temporary observation as permission to use the path later.

Allocate the name by creating the file

The page now supplies a preferred destination. When the manager starts a download, it attempts an exclusive creation:

await File(candidate).create(exclusive: true);
t._localPath = candidate;
t._ownsLocalFile = true;

Two tasks can attempt report.txt, but both cannot successfully create it. The task that encounters a collision tries report (1).txt. Numbering preserves the extension, and the task retains the path it actually acquired for subsequent resume attempts.

The useful change in responsibility is that the code choosing a destination also acquires it. The page no longer hands the queue a promise that the path will still be empty.

Creation errors also need interpretation. Treating every FileSystemException as a collision would turn a missing parent directory or a local I/O problem into an endless search for another filename. The implementation checks the entity at the candidate path after a creation error. If it is still absent, the error is propagated; an existing entity permits another name to be tried.

The ownership flag matters during cleanup as well. Removing a failed or cancelled task may delete a partial download, but only if the task created that file. A task that never acquired its destination must not delete somebody else’s file while cleaning up.

The boundary is specific: this prevents competing downloads from claiming the same initial path, and avoids directly overwriting a destination that already exists. Creation and the later open remain separate operations. It is not a claim that an arbitrary local process cannot replace the path between them.

Count writes after they complete

The old stream listener called IOSink.add() for each incoming chunk and immediately increased done. At the end of the network stream it closed the sink, but the cleanup helper swallowed close errors.

That made the progress counter ambiguous. It included bytes handed to a buffer, without waiting for the corresponding local write to finish. Using that counter as a resume offset could put the remote read position ahead of the local file. Ignoring a close error could also conceal a local failure behind a completed task.

The new loop waits for each local write before advancing progress:

while (await iterator.moveNext()) {
  if (_disposed || t._generation != generation) break;
  final chunk = iterator.current;
  await file.writeFrom(chunk);
  t.done += chunk.length;
  _progress(t);
}

A StreamIterator puts fetching and writing into one asynchronous control flow. The file is closed in finally, and a close failure becomes a task failure too. Completion is recorded only when the loop and close finish without errors and the run is still current.

Before resuming, the manager also checks that the local file length equals done. If the partial file has grown or been truncated, it refuses to append at a stale offset.

This is a length check, not an integrity check. It cannot detect replacement with different content of the same length, or prove that the remote file has remained unchanged. This change does not add hashes, remote version binding, or recovery across app restarts. Waiting for writes and close is also not a power-loss durability guarantee.

Resume must wait for cancellation to finish

Once file operations became an awaited sequence, another ordering problem needed an explicit answer: a user can cancel a download and immediately resume it.

The task may already show cancelled while the old run is still closing its iterator or file. If the next run opens the partial file immediately, its writes can overlap the old cleanup.

The fix uses two separate mechanisms:

  1. Each start captures a new _generation. Cancellation increments it too, invalidating results from the old run.
  2. The task retains its _downloadRun future. A new run waits for the previous download to finish before acquiring a client and beginning its writes.

Waiting orders access to the local file. The generation check controls which asynchronous results are still allowed to change the task. One cannot replace the other.

The same check follows await provider() when resume needs a fresh SFTP client. If cancellation happened while the provider was working, a late client cannot install itself into the manager or restart the cancelled transfer.

Test the interleaving directly

The regression cases use controlled fake SFTP streams and real temporary files. They do not depend on repeatedly running concurrent work and hoping to hit a race:

  • A pre-existing file retains its bytes, while the download goes to a numbered path.
  • Two downloads requesting the same destination finish with distinct paths, preserving both streams’ contents.
  • A download that encounters a name collision, receives some bytes, then fails resumes into its reserved path at the saved offset.
  • Immediate retry during cancellation waits for cleanup and produces the correctly concatenated file.
  • A client provider completed after cancellation cannot change the manager’s client or start a transfer.
  • A missing destination parent produces a failed task rather than a filename retry loop.

These cases are in sftp_transfer_manager_test.dart. For this article I checked the commit, implementation, and assertions; I did not rerun the Flutter tests or add a new device-testing result.

The central change is a clearer account of what a task owns. A filename is acquired through creation. Progress follows completed write operations. A new run takes over a partial file after the old run has finished with it. The UI’s completion state depends on all three happening in the right order.

✦

Comments

  • Loading…

Comments are reviewed before publishing; email is visible only to me.