Storage: multipart progress-stall detection is not suspension-aware, plus four pre-existing gaps around it #4407

Summary

This is the follow-up issue split out of the review on #4363, so that merging it (which closes #4362) does not close these with it. None of the items below is introduced by #4363; all predate it, and #4363 deliberately leaves them alone. They were found by @thisisabhash while reviewing that PR — I have verified each one against the code and say below what I did and did not verify.

Two themes: stall detection does not account for iOS app suspension (A, B, F), and three state-machine or cleanup gaps sit next to it (C, D, E).

A. Progress-stall detection is not suspension-aware

The stall deadline is an absolute DispatchTime (stallTimerQueue.asyncAfter(deadline: .now() + progressStallTimeoutSeconds, ...), StorageMultipartUploadSession.swift:253). That clock keeps running while the app is suspended, and no progress events reach a suspended app, so after a long enough suspension the deadline is already overdue the moment the app is woken. The timer then fires before, or alongside, the background session's queued part callbacks, and aborts parts nsurlsessiond was still sending or had already finished.

Separately, URLSession treats any transfer started while the app is in the background as discretionary, so iOS can hold those parts back for a long time with no progress at all — which the stall timer cannot distinguish from a dead network.

Both are the same shape: the timer measures wall-clock silence, but silence in a suspended or discretionary app is not evidence that the transfer is wedged.

Possible directions, not a proposal:

• Confirm the stall from the tasks' countOfBytesSent before aborting.
• Re-arm the timer when background events are delivered or the app returns to the foreground.

B. CompleteMultipartUpload is not protected against suspension

With #4363 in, a Complete issued during a background wake is no longer aborted by the stall timer. It still runs on the plugin's default (not background) URLSession: FoundationClientEngine.init() builds its session from URLSessionConfiguration.default (AmplifyPlugins/Core/AmplifyCredentials/CustomHttpClientEngine/FoundationClientEngine.swift:40-43). The plugin resumes the background-events continuation as soon as URLSession finishes delivering (StorageServiceSessionDelegate.urlSessionDidFinishEvents(forBackgroundURLSession:), :103-114), and nothing keeps the app awake for the request.

So if the app is suspended before S3 replies and the request then fails, or the app is terminated, the parts are left on S3 and nothing retries Complete. A beginBackgroundTask around Complete would cover most of that.

C. pause() after the last part reports .failed for an upload S3 completed

.completed is only accepted from .parts:

A pause() after the last part, held until CompleteMultipartUpload's response arrives, leaves the session in .paused. The .completed transition then throws invalidStateTransition, and the upload is reported .failed even though S3 completed it. resume() reports it again.

D. cancel() during CreateMultipartUpload reports .failed, then the upload carries on

The .created transition is applied whatever the current state is — there is no state check, unlike .completed:

So a cancel() while creation is in flight reports .failed, and then the create response lands on top of that .failed session, puts it back in .parts, and the upload proceeds to upload every part and complete.

E. A failed upload leaves its parts on S3

When a failure leaves an upload ID behind, nothing issues AbortMultipartUpload, so the parts stay in the bucket and are billed until a lifecycle rule removes them. This is pre-existing — it is also what happened with the stall timeout disabled — and #4363 does not change it, but it is worth fixing explicitly rather than relying on bucket configuration.

F. Single-part uploads have the same suspension exposure as A

StorageServiceSessionDelegate keeps a per-task stall timer (stallTimerWorkItems, :23). After the initial arm, the only thing that resets it is didSendBodyData (:230, resetting at :254 and :256). A suspended single-part upload is therefore exposed exactly as A describes, on the same absolute-deadline mechanism (:58).

What I verified, and what I did not

Verified by reading the code at #4363's head (98fd8d952): the .completed-from-.parts-only restriction (C), the unconditional .created transition (D), FoundationClientEngine's use of URLSessionConfiguration.default and the continuation resume point (B), and that didSendBodyData is the single reset path for the single-part timer (F).

Not reproduced on a device: none of A, B, C, D or F. They are read from the code, and the mechanisms follow from it, but I have no device measurement of how often any of them occurs, or of how long a suspension it takes for A to fire. @thisisabhash noted the same for A when raising it.

The one thing we do have field measurement of is the defect #4363 fixes, from #4362: a 97.8 MB multipart upload cancelled three times with Upload cancelled due to progress stall timeout on a healthy network, one of those after reaching 82%. That 82% cancellation is not explained by #4363 — the line it changes only runs once every part is .completed — which is what pointed at A in the first place.

Environment

• amplify-swift main at fix(storage): stop the progress stall timer from aborting a multipart upload with every part done #4363's base, plus fix(storage): stop the progress stall timer from aborting a multipart upload with every part done #4363.
• iOS, physical device, release configuration, background multipart uploads of 50–100 MB.