SE-0298: Async/Await: Sequences

Evolution Proposal Reviews
39

You refactored the code here to no longer require self for the teardown. In this very simplified example, this happens to work because the shutdown is a 2 step process where the second step doesn't need any information from self, so you:

  1. cancel the DispatchSource (where you still have access to self)
  2. close the file descriptor (doesn't need access to self)

Please also note that your refactoring is very fragile. If for any reason self gets captured in the closure for setCancelHandler, you'll never tear down your resources.

More importantly though, in general, this strategy will not work. Try for example creating anything with a 3 or more step shutdown process. For example if you were to create a DispatchSourceRead and a DispatchSourceWrite and you must only close the fd once both of them have executed the cancellation handler.

See below for the next simple example which is two resources that where we can at least request cancellation at the same time. Again, I'm not saying it's impossible (in deinit) but you'll need to start to divorce your state machine from the object that created the resources and create a separate state machine in another object, that makes it (in general) much harder.

import Dispatch

class ReadFDIterator: AsyncIteratorProtocol {
    typealias Element = [UInt8]

    private enum State: Equatable {
        case activated
        case cancellingEventing(readStillAlive: Bool, writeStillAlive: Bool)
        case resourcesDropped
    }

    private let fd: CInt
    private let readSource: DispatchSourceRead
    private let writeSource: DispatchSourceWrite
    private var state = State.activated

    private func cancelHandlerRead() {
        // When this is called, we regain ownership of the fd and can close it.
        // Closing it before is a use-after free.
        switch self.state {
        case .cancellingEventing(readStillAlive: let readAlive, writeStillAlive: false):
            // The write source was cancelled first, so we can go ahead and close the fd.
            assert(readAlive)
            self.state = .resourcesDropped
            close(self.fd)
        case .cancellingEventing(readStillAlive: let readAlive, writeStillAlive: true):
            assert(readAlive)
            self.state = .cancellingEventing(readStillAlive: false, writeStillAlive: true)
            // not closing fd, waiting for write to cancel
        case .activated, .resourcesDropped:
            preconditionFailure("illegal state: \(self.state)")
        }
    }
    
    private func cancelHandlerWrite() {
        // When this is called, we regain ownership of the fd and can close it.
        // Closing it before is a use-after free.
        switch self.state {
        case .cancellingEventing(readStillAlive: false, writeStillAlive: let writeAlive):
            // The read source was cancelled first, so we can go ahead and close the fd.
            assert(writeAlive)
            self.state = .resourcesDropped
            close(self.fd)
        case .cancellingEventing(readStillAlive: true, writeStillAlive: let writeAlive):
            assert(writeAlive)
            self.state = .cancellingEventing(readStillAlive: true, writeStillAlive: false)
            // not closing fd, waiting for read to cancel
        case .activated, .resourcesDropped:
            preconditionFailure("illegal state: \(self.state)")
        }
    }

    init(fileDescriptor: CInt) {
        self.fd = fileDescriptor
        // From this point on, Dispatch owns the file descriptor, we only get it back once the
        // cancel handler has been called. We cannot close the fd before.
        self.readSource = DispatchSource.makeReadSource(fileDescriptor: fileDescriptor)
        self.writeSource = DispatchSource.makeWriteSource(fileDescriptor: fileDescriptor)

        // Deliberate retain cycle to keep `self` alive until `self.cancelHandler` has been called.
        self.readSource.setCancelHandler(handler: self.cancelHandlerRead)
        self.writeSource.setCancelHandler(handler: self.cancelHandlerWrite)

        // Kick off the eventing.
        self.readSource.activate()
        self.writeSource.activate()
    }

    func next() async throws -> [UInt8]? {
        let element = Array<UInt8>?.none // do the actual work
        if element == nil {
            self.cancel() // no need to duplicate the logic in this case
        }
        return element
    }

    func cancel() {
        switch self.state {
        case .activated:
            self.state = .cancellingEventing(readStillAlive: true, writeStillAlive: true)
            self.readSource.cancel() // request cancellation
            self.writeSource.cancel()
        case .cancellingEventing, .resourcesDropped:
            () // nothing to do
        }
    }

    deinit {
        assert(self.state == .resourcesDropped)
    }
}
3 Likes