Fix recursive lock crash in NWPathMonitor AsyncStream#1135
Open
Fix recursive lock crash in NWPathMonitor AsyncStream#1135
Conversation
Guard against NaN propagation when min/max temperature values produce a zero-span range. Extract math into testable ColorTemperatureRowMath and replace stride-based gradient generation with an integer-step approach that is safe when the span is zero. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
onTermination fires while AsyncStream._Storage's internal os_unfair_lock is held. Calling cancel() synchronously from that closure risks re-entering the same lock if the Network framework's teardown path calls back into the same AsyncStream storage, causing _os_unfair_lock_recursive_abort. Deferring the cancel() to a global queue breaks any potential re-entrancy chain without changing the functional behaviour. Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
…lock crash On iOS 17+, NWPathMonitor.makeAsyncStream() has a bug where startLocked(lockedState:) holds an internal os_unfair_lock and then calls AsyncStream.Continuation.finish() from within that locked context. finish() tries to re-acquire the same lock, causing _os_unfair_lock_recursive_abort on com.apple.network.connections (confirmed in production crash report issue 1775adbe288bdbc47fda646aea5d92f2). Replace the #available(iOS 17) branch that used `for await path in monitor` (which triggers makeAsyncStream()) with our own paths() wrapper on all versions. The wrapper uses pathUpdateHandler + AsyncStream directly, bypassing the buggy Apple implementation entirely. Also removes the @available(iOS, obsoleted: 17.0) guard that was preventing paths() from being called on iOS 17+, and fixes the dispatch queue label typo (NSPathMonitor → NWPathMonitor). Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Defensive fix for a production crash (Firebase issue af3200f1b50a8163b09373952f7baa77) caused by a recursive
os_unfair_lockabort in the Network framework's interaction with Swift Concurrency.NWPathMonitor.cancel()off the AsyncStream lock context —onTerminationfires whileAsyncStream._Storage's internalos_unfair_lockis held; callingcancel()synchronously risks re-entering the same lock during Network framework teardown#available(iOS 17)branch that usedfor await path in monitor(which triggers the buggymakeAsyncStream()) with our ownpaths()wrapper on all versions, bypassing the Apple bug entirely@available(iOS, obsoleted: 17.0)guard onpaths()and fixes a dispatch queue label typo (NSPathMonitor→NWPathMonitor)The root cause is an Apple bug where
startLocked(lockedState:)holds an internal lock and then callsAsyncStream.Continuation.finish(), which tries to re-acquire the same lock.Test plan
_os_unfair_lock_recursive_abortoncom.apple.network.connections