The bug report arrived with a screen recording and no reproduction rate. Someone on the team had been checking the demo build of our short-video SDK. Publish a clip, get redirected to the grid, wait until the upload progress bar climbed somewhere between 30 and 99 percent, then hit cancel.
The clip stayed in the grid, labeled as processing.
A pull-to-refresh later, it was gone. The server never had it. The app had reported a canceled upload as a successful publication, slipped a ghost entry into the grid, and waited for reality to clean up.
The report sat in the backlog for a while because the bug refused to reproduce on demand. It always reproduced eventually. Races are patient. From that report to a merged fix took almost three months, and the fix that shipped was smaller than the one its own author built and cut within a single evening. This is the full anatomy.
The contract that should have saved us
Uploads in the SDK run as tasks inside an executor. The base task class owns a simple contract, simplified here to the parts that matter:
The routing looks defensive. Whatever happens inside upload, the task consults the cancellation flag before deciding between Done, Failed, and Canceled. The grid listens to those notifications and renders the matching state. Done means the clip is on the server. Canceled means the user pulled the plug.
Executors implement two operations against these tasks:
On paper the contract is closed. Cancel sets a flag and interrupts the work, the routing reads the flag, the grid tells the truth. The paper was wrong twice.
What cancel actually did
The executor path in the demo used a classic implementation: a map of futures on a small thread pool. Cancel looked like this:
cancel(true) means interrupt the thread running the task. It is worth pausing on what an interrupt actually is, because the entire bug hides in the gap between what people assume and what the platform promises.
An interrupt is a request, not an order. It sets a boolean on the target thread. If that thread is parked in certain blocking operations, on interruptible channels for instance, the operation aborts and the channel closes. If the thread is grinding through a CPU loop, or waiting inside a call that never checks interrupt status, nothing happens at all. The flag just sits there, politely ignored.
The cooperative behaviors are scattered across the standard library. Thread.sleep and Object.wait throw InterruptedException when the flag is set. Blocking reads on interruptible channels throw ClosedByInterruptException and close the channel. LockSupport.park wakes up and leaves the status set for you to check. A CPU loop sees nothing unless it asks. Each layer picks its own convention, and the caller has to know which one applies.
For cancellation to work, every layer between the button and the socket has to cooperate. One catch block without a rethrow breaks the chain.
Our chain had two breaks.
The poller that swallowed the interrupt
Inside the upload library, the transfer is driven by a polling loop. Send a chunk, poll the operation status, sleep, repeat until the server reports completion. The loop is old, proven in years of traffic, and was written long before anyone pushed anything cancelable through it.
The interrupt arrived while the poller was blocked mid-read. The channel did exactly what the platform promises: it threw ClosedByInterruptException.
The poller caught it.
Somewhere in that library, years ago, a developer wrote a catch block that treated a failed poll as a reason to stop looping and return a null status. It was a reasonable decision at the time. Status polls fail transiently all the time, and crashing a transfer over one bad poll would have been worse. The exception died there, in a handler that had no idea it had just eaten a cancellation.
So upload() came back the way it always does. Normal return, no exception, no signal. From the task’s point of view, the transfer had simply finished its business.
The flag nobody set
The second break was closer to home.
Remember the contract: routing checks isCanceled, and isCanceled is set by onCancel(). Somebody has to call it. The executor above never did. It called future.cancel(true), removed the task from the map, and returned. onCancel sat in the interface like a fire alarm nobody ever wired to a sensor.
Put the two failures in one timeline. The user taps cancel. The executor interrupts the thread but leaves the flag false. The poller eats the exception and lets upload() return normally. Control comes back to run(), which consults the flag, finds it false, matches the success branch, and fires notifyUploadDone. The grid renders the ghost.
The debug logging that landed with the investigation shows the exact state at the moment of routing. isCanceled=false, a normal return, not a single exception in sight. Two failures compounding into one checkmark.
Here is the part I find most instructive. Either failure, fixed alone, would have fixed the user-facing lie. If the executor had set the flag, routing would have said Canceled even on a normal return. If the poller had rethrown, the interrupt would have propagated and routing would have said Canceled through the exception path. The bug needed both holes open at once. Both were open for years, stacked on top of each other. Then a user canceled at exactly the wrong moment, and someone happened to be recording the screen.
Single-cause thinking would have found one hole and stopped.
The fix that shipped: one line, two checkpoints
The net diff that survived review is four files, 42 lines added, one removed. Most of those lines are comments and logging. The logic fits in three small moves.
Move one, in the executor, sets the flag before the storm:
One line. onCancel(null) runs before the interrupt, so the flag is already true by the time anything asynchronous wakes up. Even if every exception on the way down gets swallowed, the flag survives, because a boolean cannot be caught.
One word in the listing does quieter work: @Volatile. The flag is written on the caller’s thread, the one running cancel, and read on the worker thread inside onProgress and after upload() returns. Without the happens-before edge that @Volatile provides, the worker might go on seeing a stale false long after the caller set it. The original contract had this right from day one. The bug was never in the flag’s declaration. It was in nobody calling the method that sets it.
Move two and three live in the task, as two checkpoints on the same flag. First, inside the progress callback the library invokes during transfer:
Second, immediately after upload() returns. This one exists because of a sneaky property of the broken path: when the poller swallows the interrupt, it can exit without ever invoking the progress callback again. A checkpoint that only lives inside onProgress would never run. So the return path got its own guard:
Both checkpoints throw the same exception type, deliberately. run() already knows how to route an InterruptedException to the interrupted notification, so the fix reuses the existing plumbing instead of inventing a third signaling mechanism. The exception is thrown on our side of the library border, where nobody can swallow it.
One more line in the diff, unrelated to the race, made me smile during review. The demo screen had been picking a random author on every open: authorsList.random() became authorsList.first(). Two consecutive runs of the same scenario could render different screens, so nobody could prove anything from a screenshot. Even a race-fix commit carries a one-line determinism fix.
The layer that lived one hour
The story of the fix that did not ship is the better story.
The first version of the merge request went after the symptom where it surfaced. If the canceled upload was landing in storage as processing, update the storage status on cancel. A small patch, done, green.
The reviewer pushed back within hours. The patch, he argued, walks around the flow instead of through it. Cancel should trigger the canceled event through the task, and the controller should wait for the task to actually stop before updating anything. Otherwise you will race yourself: restart the upload, and the stale cancel status from the old task lands afterwards and kills the new one.
He was right, and he had described a bug that did not exist yet. In the same thread he asked the question that names this article: who exactly calls onCancel and sets the flag? Nobody did. A few days later he left two more notes: split the exception handling into separate catch blocks, and explain what actually differs between the demo executor and the production one.
Six days after opening the merge request, the author came back with the fortress. Seven files, 118 lines added, two removed. The flag set first, the two checkpoints, and a brand new method on the public executor interface:
A synchronous cancellation contract. The production implementation blocked on the job manager until the cancel had actually been delivered to the worker. The demo implementation used a bounded wait, future.get(2, TimeUnit.SECONDS), best effort after that. That same afternoon, an automated compatibility check had flagged a public API change in the SDK.
The fortress survived one hour. At 21:29 the same evening, sixty-four minutes after it was committed, a trim commit removed the entire API layer, minus 76 lines, and left the flag and the two checkpoints standing.
Nothing in the review thread demanded the deletion. As far as the record shows, the strongest criticism of the fortress came from its own author. If you spend an afternoon watching an automated checker itemize what a new public method costs every consumer of the SDK, deleting it yourself is a reasonable way to end the day.
What stayed in review afterwards was polish: a KDoc clarification and the catch-block split the reviewer had asked for, both landing in late July. By then a bot reported the branch was 448 commits behind develop. It was rebased and merged in early August, same shape, and shipped that same week.
The reasoning behind the trim holds up in production. The checkpoints close both holes that produced the lie, on every path, with no new surface. And there is the answer to the reviewer’s own question about what differs between the demo executor and the production one. With the checkpoints living inside the task, nothing differs where this bug is concerned. Any executor that fails to call onCancel, or delivers its interrupt late, still lands on a task that checks the flag itself and routes honestly. The fix moved the truth from the executor’s manners to the task’s own two eyes. The synchronous API answered a different question, the restart race the reviewer had predicted, and it answered it by taxing every consumer of the SDK forever. A method on a public interface is a promise with no expiry date. Two weeks in production: no ghosts in the grid, no restart races reported. The fortress was right about the danger. The one-hour deletion was right about the cost.
Why no test caught this
The obvious question deserves its own section, because the answer is structural.
The fix shipped with no test. That is not an accusation, it is the pattern. Look at how the upload pipeline is covered. UI tests run against stubs, and the stub executor’s cancel is literally fun cancel(task: UploadTask) = Unit. A no-op. In every test environment, canceling an upload does nothing at all, which means the buggy path, executor plus real threads plus real poller, is unreachable by construction.
The routing contract itself had no test either. Cancel implies onCancel, which implies the flag, which implies the interrupted notification. That chain lived in nobody’s head as a checkable assertion. When the reviewer asked, three weeks after the bug report, who exactly calls onCancel and sets the flag, he was doing the job of a missing contract test. In prose. Once. For one reviewer, in one review, that knowledge survives a week.
And the race itself is timing-shaped. The report had no reproduction rate because there wasn’t one. You needed the progress bar in the report’s own window of 30 to 99 percent, the interrupt landing mid-read, and the poller in its swallowing mood. A deterministic test for this bug would have had to fake the swallow deliberately, which nobody thinks to do until they have been bitten by it.
The takeaway is not write more tests. It is encode the contracts you already claim. If an interface says cancel must set the flag, one contract test per executor implementation would have caught this in the demo executor years ago:
Three lines of assertion. The bug was three months.
Fixes that never made it
For completeness, the dead ends beyond the one-hour layer.
Patch the polling loop. Not our code. The library ships as a binary, and forking it for one catch block trades a race we understand for a fork we would own forever.
Catch ClosedByInterruptException in the task. There is nothing to catch. The exception never crosses the library border. A fix that assumes it propagates is the bug that assumed it propagates.
Route every exception to Canceled. Network failures, storage errors, and server rejections all flow through the same call. Declaring them all canceled hides real failures behind a button the user never pressed.
Takeaways
- An interrupt is a request. Cooperation is not automatic and someone has to write it. Every catch inside a long-running loop is a place where a cancellation can die.
- Route terminal states by a flag that is set before the disruption begins, not by the presence of an exception after it. Flags survive swallowed exceptions. A boolean cannot be caught.
- Two independent silent failures can multiply into one loud lie. Either hole alone would have kept the routing honest, which is exactly why the bug survived review: every layer looked defensible on its own.
- The smallest fix that closes every observed hole beats the biggest fix that closes imagined ones. The author of the three-layer fortress deleted its strongest layer himself, an hour in, and two weeks of production have backed him.
- Encode contracts in tests, not in reviewers. If the interface promises that cancel sets a flag, assert it once per implementation. The reviewer who asked the question out loud was substituting for a three-line assertion.
The grid no longer shows ghosts. It took one flag, two checkpoints, and an author willing to delete his own favorite layer within the hour to make the truth boring again.
One Flag, Two Checkpoints: How a Canceled Upload Reported Success was originally published in ProAndroidDev on Medium, where people are continuing the conversation by highlighting and responding to this story.