fix(ios): install ubiquity observer only after emitter callback is bound#61
Open
mariusvanwyk wants to merge 1 commit into
Open
fix(ios): install ubiquity observer only after emitter callback is bound#61mariusvanwyk wants to merge 1 commit into
mariusvanwyk wants to merge 1 commit into
Conversation
The NSUbiquityIdentityDidChangeNotification observer was being registered in -init, but its block calls emitCloudAvailabilityChanged, which delegates to the codegen-generated emitOnCloudAvailabilityChanged:. That invokes a std::function event-emitter callback that is only bound later via -setEventEmitterCallback:. If iOS posts the ubiquity-change notification in the window between -init and -setEventEmitterCallback: (for example on app launch after a reboot, or when the user toggles iCloud Drive), the block invokes an empty std::function, throwing std::bad_function_call and terminating the process with SIGABRT. Move the observer installation into -setEventEmitterCallback: so it can only fire after the emitter callback is bound. -dealloc already removes the observer. The existing 'emit current availability immediately after binding' behavior is preserved. Implementation based on the proposal in kuatsu#59. Fixes kuatsu#59
✅ Deploy Preview for react-native-cloud-storage ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
merge please 🙏 |
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
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
Fixes #59.
RCTCloudStorageCloudKitregistered itsNSUbiquityIdentityDidChangeNotificationobserver in-init, but the observer's block ends up calling the codegen-generated-emitOnCloudAvailabilityChanged:, which invokes astd::functionevent-emitter callback that is only bound later via-setEventEmitterCallback:.If iOS posts the ubiquity-change notification in the window between
-initand-setEventEmitterCallback:(e.g. on app launch after a reboot, when the user signs in/out of iCloud, or when iCloud Drive is toggled), the block invokes an emptystd::function, throwingstd::bad_function_calland terminating the process withSIGABRT.Fix
Move the observer installation into
-setEventEmitterCallback:so it is only installed once the emitter callback is bound.-deallocalready removes the observer via the stored token, so no change needed there. The existing "emit current availability immediately after binding" behavior is preserved (we still callemitCloudAvailabilityChangedat the end of-setEventEmitterCallback:).This matches the implementation approach proposed by @ayousufi in #59.
Crash confirmation
Two independent reports against
react-native-cloud-storage@3.0.0on iOS (New Architecture / Turbo Modules):Reproduced on:
Testing
setEventEmitterCallback:continues to callemitCloudAvailabilityChangedat the end).Notes
NSNotificationCenter-style observer.