[Review, 2nd] SF-0023 `ProgressReporter`: Progress Reporting in Swift Concurrency

I don't understand this.

Let's set aside the async sequence for a second, and talk about SwiftUI. By my understanding, it does something like

func render() {
    withObservationTracking {
        view.body
    } onChange: {
        DispatchQueue.main.async {
            render()
        }
    }
}

(vastly oversimplified).

In this case, I think it can miss an update if the ProgressReporter.value is updated concurrently:

  • withObservationTracking called (internally calls generateAccessList)
  • view.body called
  • view.body accesses progressReporter.value, receiving 0.9 (internally, ObservationRegistrar.access is called)
  • whilst view.body continues executing, progressReporter.value is concurrently updated to 1.0 (internally, ObservationRegistrar.willSet and didSet are called, but nobody is registered to be notified yet)
  • view.body returns; the observation is established (interest in progressReporter.value) (internally _installTracking is called)
  • progressReporter.value never changes again, so onChange: is never called, so the view is never re-rendered

I think the AsyncSequence case has a similar problem — There's always a "gap" between generateAccessList calling its closure to figure out what we're interested in, and _installTracking actually ensuring we get notified of changes, into which a concurrent update of a property can slip.

Your response suggested:

/*defensive*/
for await fraction in progressValues {
  updateDisplay(fraction)
}
updateDisplayAsFinished()

But by my understanding, the async stream of observations will not finish unless

  • the iterating task is cancelled (in which case we certainly don't want to claim the operation completed)
  • theoretically if the stream's closure accessed no properties (eg. because a weak reference has dropped), the stream could notice that the autoclosure to withObservationTracking is not called, and terminate rather than suspend until the iterating task is cancelled; it appears not to handle this case.