-
Notifications
You must be signed in to change notification settings - Fork 376
fix: Remove throwing "initWithContext was not called or timed out", introduced in 5.4.0 #2408
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
Conversation
| // Check state and provide appropriate error messages | ||
| when (initState) { | ||
| InitState.FAILED -> { | ||
| throw IllegalStateException("Initialization failed. Cannot proceed.") | ||
| } | ||
| InitState.NOT_STARTED -> { | ||
| throw IllegalStateException("Must call 'initWithContext' before 'logout'") | ||
| } | ||
| InitState.IN_PROGRESS, InitState.SUCCESS -> { | ||
| // Continue - these states allow proceeding (will wait if needed) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate code, can we make a function for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good callout, i centralized all of this. including the code path for suspend and blocking.
OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt
Outdated
Show resolved
Hide resolved
OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt
Outdated
Show resolved
Hide resolved
| trigger.complete() | ||
|
|
||
| // Wait for initialization to complete (internalInit runs asynchronously) | ||
| var attempts = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the wait mechanism for the initialization seems repetitive in the test. Can we clean this up?
jkasten2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather not have waitUntilInitInternal return, but initState == InitState.IN_PROGRESS. It creates an extra inconsistent state and doesn't provide any benefit I can see.
I would rather we just wait here as long as it takes so code doesn't have to consider both states. PR #2412 does this, and only logs if it is taking longer than expected.
Description
One Line Summary
Fix race condition in initWithContext preventing concurrent initialization and premature SDK access.
Details
This PR fixes a race condition that could occur when
initWithContext()was called multiple times concurrently, or when SDK accessors were accessed before initialization completed. The fix ensures thread-safe initialization state management and prevents inconsistent SDK state.The Problem
The race condition could manifest in two scenarios:
Concurrent Initialization: If multiple threads called
initWithContext()simultaneously, both could attempt to initialize the SDK, leading to:initStatevaluesPremature Access: If SDK accessors (like
OneSignal.User,OneSignal.Notifications, etc.) were accessed beforeinitWithContext()completed, they could:NullPointerExceptionThe Solution
The fix implements proper synchronization and state management:
synchronized(initLock)aroundinitStatechecks to prevent concurrent initialization attemptsinitStateis properly set toIN_PROGRESSbefore starting initialization andSUCCESS/FAILEDafter completionCompletableDeferredto ensure consistent stateRelated Issues
Motivation
To address the race condition where concurrent initialization attempts or premature SDK access could lead to crashes or inconsistent behavior.
Scope
OneSignalImp.initWithContext()synchronizedblocksTesting
Unit testing
InitStateenum transitionsManual testing
initWithContext()calls from multiple threadsinitWithContext()Affected code checklist
Checklist
Overview
Testing
Final pass
This change is