Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SR-14875] Resuming continuations from actor contexts can hang #57222

Closed
josephlord opened this issue Jul 4, 2021 · 7 comments
Closed

[SR-14875] Resuming continuations from actor contexts can hang #57222

josephlord opened this issue Jul 4, 2021 · 7 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior.

Comments

@josephlord
Copy link

Previous ID SR-14875
Radar rdar://problem/80238311
Original Reporter @josephlord
Type Bug
Status Resolved
Resolution Duplicate

Attachment: Download

Environment

Xcode 13 beta 2. Snapshot builds 5.5 toolchain 2021-07-03, 2021-07-01, 2021-06-29, M1 Mac mini, Monterey beta2, Xcode beta 3, Monterey beta 3

Additional Detail from JIRA
Votes 1
Component/s
Labels Bug
Assignee None
Priority Medium

md5: 0f377d3ebb1ee24957d4228c49aeb885

duplicates:

  • SR-14802 Concurrency: Unstructured Task Triggering Continuation Can Hang

relates to:

  • SR-14841 Concurrency: Resuming a stored continuation from an actor does not work

Issue Description:

Note also separate hang report here: SR-14802

I've been playing with and testing creating my own AsyncSequences. The first implementation below works well, the second is highly unreliable. The only difference is that the first one returns the continuation from the actor which then gets resumed and the second one it is resumed from the actor context.

I'm not aware of anything in the specification about not firing continuations from actor context. If it really shouldn't be done there should be compile time errors if there is an attempt to resume a continuation from an actor context.

@available(macOS 12.0, iOS 15.0, *)
public struct AsyncTimerActorSequence : AsyncSequence {
    
    public typealias AsyncIterator = Iterator
    public typealias Element = Void
    
    let interval: TimeInterval
    
    public init(interval: TimeInterval) {
        self.interval = interval
    }
    
    public func makeAsyncIterator() -> Iterator {
        Iterator(interval: interval)
    }
    
    public class Iterator : AsyncIteratorProtocol {
        
        private actor InnerActor {
            private var continuations: Deque<CheckedContinuation<(), Never>> = []
            
            fileprivate func getContinuation() -> CheckedContinuation<(), Never>? {
                return continuations.popFirst()
            }
            
            fileprivate func addContinuation(_ continuation: CheckedContinuation<(), Never>) {
                continuations.append(continuation)
            }
        }
        private let safeContinuations = InnerActor()
        private var timer: Timer?
        
        fileprivate init(interval: TimeInterval) {
            let safeConts = safeContinuations
            let t = Timer(fire: .now, interval: interval, repeats: true) { _ in
                Task {
                    let continuation = await safeConts.getContinuation()
                    continuation?.resume()
                }
            }
            self.timer = t
            RunLoop.main.add(t, forMode: .default)
        }
        
        public func next() async throws -> ()? {
            await withCheckedContinuation { continuation in
                Task {
                    await safeContinuations.addContinuation(continuation)
                }
            }
            return ()
        }
        
        deinit {
            timer?.invalidate()
        }
    }
}


@available(macOS 12.0, iOS 15.0, *)
public struct AsyncTimerActorSequenceUnreliable : AsyncSequence {
    
    public typealias AsyncIterator = Iterator
    public typealias Element = Void
    
    let interval: TimeInterval
    
    public init(interval: TimeInterval) {
        self.interval = interval
    }
    
    public func makeAsyncIterator() -> Iterator {
        Iterator(interval: interval)
    }
    
    public class Iterator : AsyncIteratorProtocol {
        
        private actor InnerActor {
            private var continuations: Deque<CheckedContinuation<(), Never>> = []
            
            fileprivate func fireContinuation() {
                continuations.popFirst()?.resume()
            }
            
            fileprivate func addContinuation(_ continuation: CheckedContinuation<(), Never>) {
                continuations.append(continuation)
            }
        }
        private let safeContinuations = InnerActor()
        private var timer: Timer?
        
        fileprivate init(interval: TimeInterval) {
            let safeConts = safeContinuations
            let t = Timer(fire: .now, interval: interval, repeats: true) { _ in
                Task {
                    await safeConts.fireContinuation()
                }
            }
            self.timer = t
            RunLoop.main.add(t, forMode: .default)
        }
        
        public func next() async throws -> ()? {
            await withCheckedContinuation { continuation in
                Task {
                    await safeContinuations.addContinuation(continuation)
                }
            }
            return ()
        }
        
        deinit {
            timer?.invalidate()
        }
    }
}

To reproduce open the package in Xcode and run the AsyncTimerSequenceUnreliableActorTests repeatedly (100 times). There should be some errors. Whereas the similar AsyncTimerSequenceActorTests should be the same tests but on the working version of the sequence.
@mickeyl
Copy link

mickeyl commented Jul 5, 2021

Confirmed. For reference, here's my report: https://bugs.swift.org/browse/SR-14841

@josephlord
Copy link
Author

Not sure if this totally a dupe of the previously raised issue as the scenario I encountered was non-deterministic but if it isn’t a dupe they certainly seem closely related.

@typesanitizer
Copy link

@swift-ci create

@josephlord
Copy link
Author

I've created a simplified test case so the issue should be clearer and if it is me misunderstanding what is meant to happen that should also be visible.

This works most of the time 90 runs out of 100 or 95 times out 100 with the commented lines in the pause method uncommented (which I think shouldn't be necessary but that does depend on the withCheckedContinuation being immediately executed. When I repeated the test with a class instead of an actor I did also get some errors with this case but that is possibly to be expected to be somewhat racy (hence the wish to use an actor in this situation).

This is tested against the 5.5 Development Snapshot 2021-07-08(a) on an M1 Mac mini running Monterey beta2. Xcode beta 2. The test is within the tests for a package and I'm using the Run repeatedly options.

import XCTest

@available(iOS 15.0, macOS 12.0, *)
actor SUTActor {
    var continuation: CheckedContinuation<(),Never>?
    var canGo = false
    
    func pause() async {
        if canGo { return }
        await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) -> Void in
//            if canGo {
//                continuation.resume(returning: ())
//            } else {
                self.continuation = continuation
//            }
        }
    }
    
    func go() {
        canGo = true
        continuation?.resume()
    }
}

@available(iOS 15.0, macOS 12.0, *)
final class ActorContinuationTests : XCTestCase {
    func testPauseGo() {
        let sut = SUTActor()
        let exp = expectation(description: "Will resume")
        Task.detached(priority: .high) {
            await sut.pause()
            exp.fulfill()
        }
        Task.detached(priority: .default) {
            await sut.go()
        }
        waitForExpectations(timeout: 0.5, handler: nil)
        _ = sut // Ensure lifetime sufficient
    }
}

@josephlord
Copy link
Author

This is still happening in beta 3 (Xcode and macOS Monterey). I'm also seeing it (although less frequently in the iOS Simulator in Xcode beta3). All tested on M1 Mac only.

This feels fairly critical to me I'm going to try to raise it on the Swift forums to see if I can get some attention for it.

@typesanitizer
Copy link

From what I can tell, the example with `SUTActor` should work if you do not explicitly specify the priorities and are using macOS 12 beta 3 (or using a recent toolchain snapshot), which should include this patch: #37939

Can you confirm if that's the case?

@typesanitizer
Copy link

This is the same issue as SR-14802, I'll add more details there in a bit.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mickeyl @josephlord @typesanitizer