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

Lock async stream inits #11

Merged
merged 2 commits into from
Oct 26, 2023
Merged

Lock async stream inits #11

merged 2 commits into from
Oct 26, 2023

Conversation

stephencelis
Copy link
Member

Fixes #10.

@mansatgigset Can you take this branch for a spin to confirm it fixes the issue for you?

@mansatgigset
Copy link

I have tested the branch, and it seems to work fine. Note though, that this crash is quite random and infrequent on later OS versions, and I don't currently have access to run this on the older Intel macOS 12.x system (or even know if that crash is related to this). I think this fix is valid no matter, so please go ahead and merge and create a new release.

@stephencelis
Copy link
Member Author

@mansatgigset Would be happy to add an availability check if we can track down the runtimes involved. Any idea which are affected, or when the fix was applied?

@mansatgigset
Copy link

Well, the crash we have seen on macOS 12.x (Intel), that we can't debug (due to the difficulty to debug older macOS versions), gave this short stack trace:

Exception Type:        EXC_BAD_INSTRUCTION (SIGILL)
Exception Codes:       0x0000000000000001, 0x0000000000000000
Exception Note:        EXC_CORPSE_NOTIFY

Termination Reason:    Namespace SIGNAL, Code 4 Illegal instruction: 4

Thread 4 Crashed:
0   libswiftCore.dylib            	0x00007ff828033245 _assertionFailure(_:_:file:line:flags:) + 421 (AssertCommon.swift:132)
1   libswift_Concurrency.dylib    	0x00007ffb34060b64 AsyncStream._Storage.next(_:) + 532 (AsyncStreamBuffer.swift:253)
2   libswift_Concurrency.dylib    	0x00007ffb340735ec partial apply for closure apple/swift-async-algorithms#2 in AsyncStream._Storage.next() + 188 (<compiler-generated>:0)
3   libswift_Concurrency.dylib    	0x00007ffb340742e1 thunk for @callee_guaranteed @async () -> (@out A?) + 1

Which was a result of an assertion in Apple's code base in previous releases.

See swiftlang/swift#69367

The other crash was actually caught when debugging on macOS using macOS 14.0, but I'm fairly sure I have seen it on iOS 16.x simulators as well.

It was the latter (and longer stack-trace) that had your type erasers in them (the one updated in this PR), hence why we suspected they might a reason behind the crash.

@stephencelis stephencelis merged commit 6155400 into main Oct 26, 2023
7 checks passed
@stephencelis stephencelis deleted the lock-async-stream-inits branch October 26, 2023 21:34
@Mika5652
Copy link

Mika5652 commented Jul 12, 2024

@stephencelis Hello, is it possible to cherry-pick these changes to the version below 1.0.0.? I am working on quite a big project, where we still have TCA 0.59.0. I started using your extension eraseToStream and I am now afraid of crashes when I found this fix. I didn't notice any crashes at this time, but this can be because these changes are not released yet and this can be problematic because our app uses around 300k users. Yes, I can copy your fix into our project, but it seems better for me to not duplicate logic. Thanks for your reply!

stephencelis added a commit that referenced this pull request Aug 5, 2024
* Lock async stream inits

Fixes #10.

* wip
@stephencelis
Copy link
Member Author

@Mika5652 Sorry for the delay, we lost track of this. Just cut a 0.1.2 with this fix cherry-picked here: https://github.com/pointfreeco/swift-concurrency-extras/releases/tag/0.1.2

@Mika5652
Copy link

Mika5652 commented Aug 5, 2024

@stephencelis It's alright. I know you have a lot of work. Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Potential crash
4 participants