An (NS)Operation subclass for async operations

AsyncOperation.swift

import Foundation

/// An abstract class that makes building simple asynchronous operations easy.
/// Subclasses must override `main()` to perform any work and call `finish()`
/// when they are done. All `NSOperation` work will be handled automatically.
///
/// Source/Inspiration: https://stackoverflow.com/a/48104095/116862 and https://gist.github.com/calebd/93fa347397cec5f88233
open class AsyncOperation: Operation {
    public init(name: String? = nil) {
        super.init()
        self.name = name
    }

    /// Serial queue for making state changes atomic under the constraint
    /// of having to send KVO willChange/didChange notifications.
    private let stateChangeQueue = DispatchQueue(label: "com.olebegemann.AsyncOperation.stateChange")

    /// Private backing store for `state`
    private var _state: Atomic<State> = Atomic(.ready)

    /// The state of the operation
    private var state: State {
        get {
            return _state.value
        }
        set {
            // A state mutation should be a single atomic transaction. We can't simply perform
            // everything on the isolation queue for `_state` because the KVO willChange/didChange
            // notifications have to be sent from outside the isolation queue. Otherwise we would
            // deadlock because KVO observers will in turn try to read `state` (by calling
            // `isReady`, `isExecuting`, `isFinished`. Use a second queue to wrap the entire
            // transaction.
            stateChangeQueue.sync {
                // Retrieve the existing value first. Necessary for sending fine-grained KVO
                // willChange/didChange notifications only for the key paths that actually change.
                let oldValue = _state.value
                guard newValue != oldValue else {
                    return
                }
                willChangeValue(forKey: oldValue.objcKeyPath)
                willChangeValue(forKey: newValue.objcKeyPath)
                _state.mutate {
                    $0 = newValue
                }
                didChangeValue(forKey: oldValue.objcKeyPath)
                didChangeValue(forKey: newValue.objcKeyPath)
            }
        }
    }

    /// Mirror of the possible states an (NS)Operation can be in
    private enum State: Int, CustomStringConvertible {
        case ready
        case executing
        case finished

        /// The `#keyPath` for the `Operation` property that's associated with this value.
        var objcKeyPath: String {
            switch self {
            case .ready: return #keyPath(isReady)
            case .executing: return #keyPath(isExecuting)
            case .finished: return #keyPath(isFinished)
            }
        }

        var description: String {
            switch self {
            case .ready: return "ready"
            case .executing: return "executing"
            case .finished: return "finished"
            }
        }
    }

    public final override var isAsynchronous: Bool { return true }

    open override var isReady: Bool {
        return state == .ready && super.isReady
    }

    public final override var isExecuting: Bool {
        return state == .executing
    }

    public final override var isFinished: Bool {
        return state == .finished
    }

    // MARK: - Foundation.Operation
    public final override func start() {
        guard !isCancelled else {
            finish()
            return
        }
        state = .executing
        main()
    }

    // MARK: - Public

    /// Subclasses must implement this to perform their work and they must not call `super`.
    /// The default implementation of this function traps.
    open override func main() {
        preconditionFailure("Subclasses must implement `main`.")
    }

    /// Call this function to finish an operation that is currently executing.
    /// State can also be "ready" here if the operation was cancelled before it started.
    public final func finish() {
        if isExecuting || isReady {
            state = .finished
        }
    }

    open override var description: String {
        return debugDescription
    }

    open override var debugDescription: String {
        return "\(type(of: self)) — \(name ?? "nil") – \(isCancelled ? "cancelled" : String(describing: state))"
    }
}

Atomic.swift

import Dispatch

/// A wrapper for atomic read/write access to a value.
/// The value is protected by a serial `DispatchQueue`.
public final class Atomic<A> {
    private var _value: A
    private let queue: DispatchQueue

    /// Creates an instance of `Atomic` with the specified value.
    ///
    /// - Paramater value: The object's initial value.
    /// - Parameter targetQueue: The target dispatch queue for the "lock queue".
    ///   Use this to place the atomic value into an existing queue hierarchy
    ///   (e.g. for the subsystem that uses this object).
    ///   See Apple's WWDC 2017 session 706, Modernizing Grand Central Dispatch
    ///   Usage (https://developer.apple.com/videos/play/wwdc2017/706/), for
    ///   more information on how to use target queues effectively.
    ///
    ///   The default value is `nil`, which means no target queue will be set.
    public init(_ value: A, targetQueue: DispatchQueue? = nil) {
        _value = value
        queue = DispatchQueue(label: "com.olebegemann.Atomic", target: targetQueue)
    }

    /// Read access to the wrapped value.
    public var value: A {
        return queue.sync { _value }
    }

    /// Mutations of `value` must be performed via this method.
    ///
    /// If `Atomic` exposed a setter for `value`, constructs that used the getter
    /// and setter inside the same statement would not be atomic.
    ///
    /// Examples that would not actually be atomic:
    ///
    ///     let atomicInt = Atomic(42)
    ///     // Calls getter and setter, but value may have been mutated in between
    ///     atomicInt.value += 1
    ///
    ///     let atomicArray = Atomic([1,2,3])
    ///     // Mutating the array through a subscript causes both a get and a set,
    ///     // acquiring and releasing the lock twice.
    ///     atomicArray[1] = 42
    ///
    /// See also: https://github.com/ReactiveCocoa/ReactiveSwift/issues/269
    public func mutate(_ transform: (inout A) -> Void) {
        queue.sync {
            transform(&_value)
        }
    }
}

extension Atomic: Equatable where A: Equatable {
    public static func ==(lhs: Atomic, rhs: Atomic) -> Bool {
        return lhs.value == rhs.value
    }
}

extension Atomic: Hashable where A: Hashable {
    public func hash(into hasher: inout Hasher) {
        hasher.combine(value)
    }
}

Hi @ole 👋 Thanks for sharing this implementation; we've seen with both this implementation and the one it's based off of by Caleb that there are EXC_BAD_ACCESS on didChangeValue(forKey: newValue.objcKeyPath) (https://gist.github.com/ole/5034ce19c62d248018581b1db0eabb2b#file-asyncoperation-swift-L46) due to Attempted to dereference garbage pointer 0x41.

I'm curious if you have an idea why since I'd assume it's protected within the stateQueue

@rob-keepsafe Yes, I have seen similar crashes from time to time, though usually SIGSEGV rather than EXC_BAD_ACCESS. But the backtraces do look similar to yours. I don't have any idea what causes it.

There have been quite a number of multithreading bugs in Swift's KVO overlay up until Swift 5.0, so I have a vague hope that building with Xcode 11/Swift 5.1 will fix things — although we did install the workaround mentioned in the linked PR, so maybe those shouldn't be an issue anyway.

Please let me know if you find something.

Thanks for the quick reply and extra information, @ole 🙇‍♂️ We're also using the KVO workaround (since it corrupted every operation across the entire sandbox/3rd party libs without it). I might file a radar for this one since it seems pretty overkill to guard it like this and yet KVO still has threading issues.

@ole @iwasrobbed-ks If there is a chance, could you please link me to the application layer KVO workaround that you referred to in your comments? It looks like the PR that was linked is not an app layer fix as I understand. I'm using Swift 5 with Xcode 12.5.1 and still seeing this issue. Thank you!

@LeonidKokhnovich If I remember correctly (it's been a while), I was referring specifically to this comment: swiftlang/swift#20103 (comment)

The only safe work around is to create a main.swift file, like suggested above #20103 (comment).

E.g.

1. Remove the @UIApplicationMain decorator from your AppDelegate

2. Create a main.swift file:

import UIKit
import Foundation

private class CrashWorkaround: NSObject {
    @objc dynamic var test: String = ""

    /// More info: https://github.com/apple/swift/pull/20103
    fileprivate static func workaroundSwiftKVOCrash() {
        assert(Thread.isMainThread)
        let obj = CrashWorkaround()
        let observer = obj.observe(\.test, options: [.new]) { (_, _) in
            fatalError("Shouldn't be called, value doesn't change")
        }
        observer.invalidate()
    }
}

/// This needs to execute before anything can register KVO in the background
/// so the best way to do this is before the application ever runs
CrashWorkaround.workaroundSwiftKVOCrash()
UIApplicationMain(CommandLine.argc, CommandLine.unsafeArgv, nil, NSStringFromClass(AppDelegate.self))