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

Fix Reachability Safety #3684

Merged
merged 2 commits into from
Feb 7, 2023
Merged

Fix Reachability Safety #3684

merged 2 commits into from
Feb 7, 2023

Conversation

jshier
Copy link
Contributor

@jshier jshier commented Feb 6, 2023

Issue Link πŸ”—

#3676

Goals ⚽

This PR updates our integration of SCNetworkReachabilityContext to use a weak box class and fully integrate with the framework's retain and release closures. This follows the same logic as Reachability.swift.

Implementation Details 🚧

This PR adds a WeakManager type to wrap a weak reference to the NetworkReachabilityManager and updates the context memory management closures to properly retain and release the values.

Testing Details πŸ”

No tests added, wasn't able to find a way to trigger the reported crash. Since this logic matches that of the other popular reachability library I'm confident it's correct.

@jshier jshier merged commit 337b29a into master Feb 7, 2023
@jshier jshier deleted the bug/reachability-safety branch February 7, 2023 02:02
@jshier jshier added this to the 5.7.0 milestone May 9, 2023
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.

1 participant