[swift-evolution] [Concurrency] Fixing race conditions in async/await example

Howard Lovatt howard.lovatt at gmail.com
Fri Aug 25 02:34:07 CDT 2017


Using a Future library, see below, you can do what you want. In particular
a future that is cancellable is more powerful that the proposed
async/await. Here is an extended example of a typical UI task including
users cancelling tasks that is written using a future library (see below):

    @IBOutlet weak var timeButtonWasPressed: NSTextField!

    static func progress(_ window: NSWindow, _ progress:
NSProgressIndicator) -> Future<String> {
        return AsynchronousFuture(timeout: .seconds(3)) { isCancelled ->
String in // Timeout set to 3 seconds to ensure normal progress closes the
window before timeout does.
            defer { // Make sure the window always closes.
                Thread.executeOnMain {
                    window.close()
                }
            }
            var isFinished = false
            var isClosed = false
            while !(isFinished || isClosed || isCancelled()) {
                Thread.sleep(forTimeInterval: 0.1) // Would do real work
here!
                Thread.executeOnMain {
                    guard window.isVisible else { // Check if user has
closed the window.
                        isClosed = true
                        return
                    }
                    progress.increment(by: 1)
                    if progress.doubleValue >= progress.maxValue { // Check
if work done.
                        isFinished = true
                    }
                }
            }
            if isClosed || isCancelled() { // Cancelled by user closing
window, by call to `cancel`, or by timeout
                throw CancelFuture.cancelled
            }
            return "\(DispatchTime.now().uptimeNanoseconds)"
        }
    }

    var windowOrigin = NSPoint(x: 0, y: NSScreen.main?.visibleFrame.height
?? 0) // First window in top left of screen.

    @IBAction func buttonPushed(_ _: NSButton) {
        let progressFrame = NSRect(x: 10, y: 10, width: 200, height: 100)
// On main thread; pop up a progress window.
        let progress = NSProgressIndicator(frame: progressFrame)
        progress.minValue = 0
        progress.maxValue = 20
        progress.isIndeterminate = false
        let windowFrame = NSRect(x: 0, y: 0, width: progressFrame.width +
20, height: progressFrame.height + 20)
        let window = NSWindow(contentRect: windowFrame, styleMask:
[.titled, .closable], backing: .buffered, defer: false)
        window.contentView?.addSubview(progress)
        windowOrigin = window.cascadeTopLeft(from: windowOrigin) //
Position window.
        window.isReleasedWhenClosed = false // Needed to keep ARC happy!
        window.orderFront(self) // Display the window but don't give it the
focus.
        let _ = AsynchronousFuture { _ -> Void in // Runs on global default
queue.
            let creationTime = ViewController.progress(window,
progress).get ?? "Cancelled" // Progress bar with close to allow user
cancellation and finishes automatically after 2 seconds to allow
cancellation or many button presses to pop up other progress windows.
            Thread.executeOnMain {
                self.timeButtonWasPressed.stringValue = creationTime
            }
        }
    }

The above pops up a window with a progress bar in it every time the user
hits a button (buttonPushed), once the progress bar has completed or is
cancelled the main UI is updated, including noting cancellation by the
user. It can popup multiple windows and cancel them in any order.

This is easier to do with a future library than the proposed async/await
because the library has concepts of: cancellation, timeout, and control
over which queue routines run on.

Future library below:

//
//  main.swift
//  Future
//  Version 0.1
//
//  Created by Howard Lovatt on 22/8/17.
//  Copyright © 2017 Howard Lovatt.
//  This work is licensed under a Creative Commons Attribution 4.0
International License, http://creativecommons.org/licenses/by/4.0/.
//

import Foundation

/// - note:
///   - Written in GCD but execution service would be abstracted for a
'real' version of this proposed `Future`.
///   - It might be necessary to write an atomic class/struct and use it
for _status and isCancelled in CalculatingFuture; see comments after
property declarations.
///   - If _status and isCancelled in CalculatingFuture where atomic then
future would be thread safe.

extension Thread {
    /// Run the given closure on the main thread (thread hops to main) and
*wait* for it to complete before returning its value; useful for updating
and reading UI components.
    /// Checks to see if already executing on the main thread and if so
does not change to main thread before executing closure, since changing to
main when already on main would cause a deadlock.
    /// - note: Not unique to `Future`, hence an extension on `Thread`.
    static func executeOnMain<T>(closure: @escaping () -> T) -> T {
        var result: T?
        if Thread.isMainThread {
            result = closure()
        } else {
            DispatchQueue.main.sync {
                result = closure()
            }
        }
        return result!
    }
}

/// All possible states for a `Future`; a future is in exactly one of these.
enum FutureStatus<T> {
    /// Currently running or waiting to run; has not completed, was not
cancelled, has not timed out, and has not thrown.
    case running


    /// Ran to completion; was not cancelled, did not timeout, and did not
throw, no longer running.
    case completed(result: T)


    /// Was cancelled, timed out, or calculation threw an exception; no
longer running.
    case threw(error: Error)
}

/// An error that signals the future was cancelled.
enum CancelFuture: Error {
    /// Should be thrown by a future's calculation when requested to do so
via its `isCancelled` argument (which arises if the future is cancelled or
if the future times out).
    case cancelled
}

/// Base class for futures; acts like a future that was cancelled, i.e. no
result and threw `CancelFuture.cancelled`.
/// - note:
///   - You would normally program to `Future`, not one of its derived
classes, i.e. arguments, return types, properties, etc. typed as `Future`.
///   - Futures are **not** thread safe; i.e. they cannot be shared between
threads though their results can and they themselves can be inside any
single thread.
///   - This class is useful in its own right; not just a base class, but
as a future that is known to be cancelled.
class Future<T> {
    /// The current state of execution of the future.
    /// - note:
    ///   - The status is updated when the future's calculation finishes;
therefore there will be a lag between a cancellation or a timeout and
status reflecting this.
    ///   - This status lag is due to the underlying thread system provided
by the operating system that typically does not allow a running thread to
be terminated.
    ///   - Because status can lag cancel and timeout; prefer get over
status, for obtaining the result of a future and if detailed reasons for a
failure are not required.
    ///   - Status however offers detailed information if a thread
terminates by throwing (including cancellation and time out) and is
therefore very useful for debugging.
    /// - note: In the case of this base class, always cancelled; returns
`.threw(error: CancelFuture.cancelled)`.
    var status: FutureStatus<T> {
        return .threw(error: CancelFuture.cancelled)
    }


    /// Wait until the value of the future is calculated and return it; if
future timed out, if future was cancelled, or if calculation threw, then
return nil.
    /// The intended use of this property is to chain with the nil
coalescing operator, `??`, to provide a default, a retry, or an error
message in the case of failure.
    /// - note:
    ///   - Timeout is only checked when `get` is called.
    ///   - If a future is cancelled or times out then get will
subsequently return nil; however it might take some time before status
reflects this calculation because status is only updated when the
calculation stops.
    /// - note: In the case of this base class, always return nil.
    var get: T? {
        return nil
    }


    /// Cancel the calculation of the future; if it has not already
completed.
    /// - note:
    ///   - Cancellation should cause `CancelFuture.cancelled` to be thrown
and hence the future's status changes to `threw` ('should' because the
calculation can ignore its `isCancelled` argument or throw some other
error).
    ///   - `isCancelled` is automatically checked on entry and exit to the
calculation and therefore status will update before and after execution
even if the calculation ignores its argument.
    ///   - Cancellation will not be instantaneous and therefore the
future's status will not update immediately; it updates when the
calculation terminates (either by returning a value or via a throw).
    ///   - If a future timeouts, it cancels its calculation.
    ///   - If the future's calculation respects its `isCancelled` argument
then a timeout will break a deadlock.
    ///   - If a future is cancelled by either cancel or a timeout,
subsequent calls to `get` will return nil; even if the calculation is still
running and hence status has not updated.
    /// - note: In the case of this base class, cancel does nothing since
this future is always cancelled.
    func cancel() {}
}

/// A future that calculates its value on the given queue asynchronously
(i.e. its init method returns before the calculation is complete) and has
the given timeout to bound the wait time when `get` is called.
final class AsynchronousFuture<T>: Future<T> {
    private var _status = FutureStatus<T>.running // Really like to mark
this volatile and atomic (it is written in background thread and read in
foreground)!


    override var status: FutureStatus<T> {
        return _status
    }


    private let group = DispatchGroup()


    private let timeoutTime: DispatchTime


    private var isCancelled = false // Really like to mark this volatile
(it is a bool so presumably atomic, but it is set in forground thread and
read in background)!


    /// - note: The default queue is the global queue with default quality
of service.
    /// - note:
    ///   Regarding the `timeout` argument:
    ///   - Timeout starts from when the future is created, not when `get`
is called.
    ///   - The time used for a timeout is processor time; i.e. it excludes
time when the computer is in sleep mode.
    ///   - The default timeout is 1 second.
    ///   - If the calculation times out then the calculation is cancelled.
    ///   - The timeout is only checked when `get` is called; i.e. the
calculation will continue for longer than timeout, potentially
indefinitely, if `get` is not called.
    ///   - Also see warning below.
    /// - warning:
    ///   Be **very** careful about setting long timeouts; if a deadlock
occurs it is diagnosed/broken by a timeout occurring!
    ///   If the calculating method respects its `isCancelled` argument a
timeout will break a deadlock, otherwise it will only detect a deadlock.
    init(queue: DispatchQueue = .global(), timeout: DispatchTimeInterval =
.seconds(1), calculation: @escaping (_ isCancelled: () -> Bool) ->
FutureStatus<T>) {
        self.timeoutTime = DispatchTime.now() + timeout
        super.init() // Have to complete initialization before result can
be calculated.
        queue.async { // Deliberately holds a strong reference to self, so
that a future can be side effecting.
            self.group.enter()
            defer {
                self.group.leave()
            }
            if self.isCancelled { // Future was cancelled before execution
began.
                self._status = .threw(error: CancelFuture.cancelled)
                return
            }
            self._status = calculation {
                self.isCancelled // Pass `isCancelled` to `calculation`
(via a closure so that it isn't copied and therefore reflects its current
value).
            }
            if self.isCancelled { // Future was cancelled during execution.
                self._status = .threw(error: CancelFuture.cancelled)
            }
        }
    }


    /// See above `init` for description.
    /// This `init` accepts a closure that returns a `T`; the above
`init`'s closure returns a `FutureStatus<T>`.
    /// This `init`'s closure is wrapped to return a `FutureStatus<T>` and
this `init` calls the above `init`.
    convenience init(queue: DispatchQueue = .global(), timeout:
DispatchTimeInterval = .seconds(1), calculation: @escaping (_ isCancelled:
() -> Bool) throws -> T) {
        self.init(queue: queue, timeout: timeout) { isCancelled ->
FutureStatus<T> in
            var resultOrError: FutureStatus<T>
            do {
                resultOrError = .completed(result: try
calculation(isCancelled))
            } catch {
                resultOrError = .threw(error: error)
            }
            return resultOrError
        }
    }


    /// See `init` 2 above for description.
    /// This `init` accepts a closure that accepts no arguments, unlike the
closures for the other `init`s that accept `isCancelled`, and returns a
`(T?, Error?)`; the `init`' 2 above's closure returns a `FutureStatus<T>`.
    /// This `init`'s closure is wrapped to return a `FutureStatus<T>` and
this `init` calls the `init` 2 above.
    convenience init(queue: DispatchQueue = .global(), timeout:
DispatchTimeInterval = .seconds(1), calculation: @escaping () -> (T?,
Error?)) {
        self.init(queue: queue, timeout: timeout) { _ -> FutureStatus<T> in
            var resultOrError: FutureStatus<T>
            let (result, error) = calculation()
            if error == nil {
                resultOrError = .completed(result: result!)
            } else {
                resultOrError = .threw(error: error!)
            }
            return resultOrError
        }
    }


    override var get: T? {
        guard !isCancelled else { // Catch waiting for a cancel to actually
happen.
            return nil
        }
        while true { // Loop until not running, so that after a successful
wait the result can be obtained.
            switch _status {
            case .running:
                switch group.wait(timeout: timeoutTime) { // Wait for
calculation completion.
                case .success:
                break // Loop round and test status again to extract result
                case .timedOut:
                    isCancelled = true
                    return nil
                }
            case .completed(let result):
                return result
            case .threw(_):
                return nil
            }
        }
    }


    override func cancel() {
        switch _status {
        case .running:
            isCancelled = true
        case .completed(_):
        return // Cannot cancel a completed future.
        case .threw(_):
            return // Cannot cancel a future that has timed out, been
cancelled, or thrown.
        }
    }
}

/// A future that doesn't need calculating, because the result is already
known.
final class KnownFuture<T>: Future<T> {
    private let result: T


    override var status: FutureStatus<T> {
        return .completed(result: result)
    }


    init(_ result: T) {
        self.result = result
    }


    override var get: T? {
        return result
    }
}

/// A future that doesn't need calculating, because it is known to fail.
final class FailedFuture<T>: Future<T> {
    private let _status: FutureStatus<T>


    override var status: FutureStatus<T> {
        return _status
    }


    init(_ error: Error) {
        _status = .threw(error: error)
    }
}


  -- Howard.

On 24 August 2017 at 14:26, Maxim Veksler via swift-evolution <
swift-evolution at swift.org> wrote:

> I think that the solution you are describing is how RxSwift (ReactiveX)
> solves this problem.
>
> I believe Rx, like many other higher level abstractions would benefit from
> async, actors behind the scenes, as an implementation detail.
>
> ‫בתאריך יום ד׳, 23 באוג׳ 2017 ב-20:41 מאת ‪Joe Groff via swift-evolution‬‏
> <‪swift-evolution at swift.org‬‏>:‬
>
>>
>> On Aug 19, 2017, at 4:56 AM, Jakob Egger via swift-evolution <
>> swift-evolution at swift.org> wrote:
>>
>> I've read async/await proposal, and I'm thrilled by the possibilities.
>> Here's what I consider the canonical example:
>>
>> @IBAction func buttonDidClick(sender:AnyObject) {
>>   beginAsync {
>>     let image = await processImage()
>>     imageView.image = image
>>   }
>> }
>>
>> This is exactly the kind of thing I will use async/await for!
>>
>> But while this example looks extremely elegant, it would suffer from a
>> number of problems in practice:
>>
>> 1. There is no guarantee that you are on the main thread after `await
>> processImage()`
>> 2. There is no way to cancel processing
>> 3. Race Condition: If you click the button a second time before
>> `processImage()` is done, two copies will run simultaneously and you don't
>> know which image will "win".
>>
>> So I wondered: What would a more thorough example look like in practice?
>> How would I fix all these issues?
>>
>> After some consideration, I came up with the following minimal example
>> that addresses all these issues:
>>
>> class ImageProcessingTask {
>>   var cancelled = false
>>   func process() async -> Image? { … }
>> }
>>
>> var currentTask: ImageProcessingTask?
>> @IBAction func buttonDidClick(sender:AnyObject) {
>>   currentTask?.cancelled = true
>>   let task = ImageProcessingTask()
>>   currentTask = task
>>   beginAsync {
>>     guard let image = await task.process() else { return }
>>     DispatchQueue.main.async {
>>       guard task.cancelled == false else { return }
>>       imageView.image = image
>>     }
>>   }
>> }
>>
>> If my example isn't obvious, I've documented my thinking (and some
>> alternatives) in a gist:
>> https://gist.github.com/jakob/22c9725caac5125c1273ece93cc2e1e7
>>
>> Anyway, this more realistic code sample doesn't look nearly as nice any
>> more, and I actually think this could be implemented nicer without
>> async/await:
>>
>> class ImageProcessingTask {
>>   var cancelled = false
>>   func process(completionQueue: DispatchQueue, completionHandler:
>> (Image?)->()) { … }
>> }
>> @IBAction func buttonDidClick(sender:AnyObject) {
>>   currentTask?.cancelled = true
>>   let task = ImageProcessingTask()
>>   currentTask = task
>>   task.process(completionQueue: DispatchQueue.main) { (image) in
>>     guard let image = image else { return }
>> guard task.cancelled == false else { return }
>> imageView.image = image
>>   }
>> }
>>
>> So I wonder: What's the point of async/await if it doesn't result in
>> nicer code in practice? How can we make async/await more elegant when
>> calling from non-async functions?
>>
>>
>> Yeah, it's important to understand that coroutines don't directly offer
>> any form of coordination; they only let you thread execution nicely through
>> existing coordination mechanisms. IBActions by themselves don't offer any
>> coordination, so anything more than fire-and-forget is still going to
>> require explicit code. There are some interesting approaches you still
>> might be able to explore to make this kind of thing nicer; for instance, if
>> buttonDidClick didn't directly trigger the task, but instead communicated
>> with a coroutine via synchronous channels in the style of Go, then that
>> coroutine could be responsible for filtering multiple click events, and
>> could also listen for cancellation events. The actor model Chris proposes
>> in his document could conceivably let you wrap up that low-level channel
>> management in a nice OO-looking wrapper.
>>
>> -Joe
>>
>> _______________________________________________
>> swift-evolution mailing list
>> swift-evolution at swift.org
>> https://lists.swift.org/mailman/listinfo/swift-evolution
>>
>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution at swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170825/231bfb25/attachment.html>


More information about the swift-evolution mailing list