[Fix] Prevent deadlocks when user manager runs its startup tasks#1559
Merged
[Fix] Prevent deadlocks when user manager runs its startup tasks#1559
Conversation
Only lock when reading and writing from the dictionary, to minimize amount of locking when unnecessary
*Calling the `user` property of the `OneSignalUserManagerImpl` will return a mock user, the user instance, or call start(). * In some methods, if there is no user instance, don't trigger a start() call, to prevent deadlocks if these methods are called by start() itself. * Currently, only the properties model and subscription model may have update enqueued at the end of the start() method to enqueue any changes such as timezone and app version for the user instance, which exists by this point. * The other refactors are for safety.
nan-li
force-pushed
the
fix/user_module_deadlocks
branch
from
1329bd1 to
96abd8b
Compare
jinliu9508
reviewed
May 13, 2025
iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift
Show resolved
Hide resolved
iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl.swift
Show resolved
Hide resolved
Contributor
Author
|
Admittedly, these gotchas and details to watch out for are getting a bit gnarly. The user module can definitely benefit from some debt refactoring work. |
jinliu9508
approved these changes
May 22, 2025
This was referenced May 27, 2025
Merged
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
2 participants
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.
Description
One Line Summary
Prevent deadlocks in the user module after making synchronization changes.
Details
Motivation
OneSignalUserManagerImpl.start()methodScope
start()method which triggers the model > store > listener flow. Eventually, it was triggering thestart()method again, resulting in a deadlock.start()to generate a user instance. Currently, only the properties model and subscription model may have update enqueued at the end of the start() method to enqueue any changes such as timezone and app version for the user instance, which exists by this point. The other no-op refactors are for safety / improvement.Testing
Unit testing
None
Manual testing
iPhone 13 on iOS 18.4.1
Tested version upgrade, timezone upgrade, other normal flows
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is