Repository navigation
fix: [SDK-5538] stop the preferences write loop from spinning after cancellation - #2833
Closed
abdulraqeeb33 wants to merge 1 commit into
Closed
abdulraqeeb33 wants to merge 1 commit into
abdulraqeeb33 wants to merge 1 commit into
Conversation
…ancellation The loop caught Throwable, which includes CancellationException, so once its scope was cancelled every pass logged an error, applied an empty edit and looped again without ever suspending. Only reachable through resetForTest() today, where it ran behind every Robolectric spec that initialized the SDK and kept QueuedWork.waitToFinish from ever draining. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff). Changed Files Coverage
Overall (aggregate gate)2/2 touched executable lines covered (100.0% — requires ≥ 80%) |
Contributor
Author
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.
Description
One Line Summary
The
PreferencesServicewrite loop stops when its coroutine scope is cancelled instead of spinning on the error path.Details
Motivation
Found while fault-testing init for SDK-5538.
doWorkAsynccatchesThrowable, which includesCancellationException. Once the scope is cancelled,delayandwaitForWakethrow on every pass, the catch logs an ERROR, and the loop runs again without ever suspending: an emptyeditor.apply()plus a log line per iteration, forever. The repro test counted 22,723 iterations in half a second.Today the only thing that cancels that scope is
OneSignalDispatchers.resetForTest(), so this is a test-infrastructure problem rather than a production one: it has been spinning behind every Robolectric spec that initializes the SDK, and in one run it keptQueuedWork.waitToFinishfrom ever draining and hung the test JVM. The fix is one line and correct regardless of who cancels.Scope
PreferencesService.doWorkAsync: rethrowCancellationExceptionbefore the generic catch.Also included in the SDK-5538 stack (#2803), where the fault tests need it; this PR lets it land on
mainon its own. The two are identical and merge cleanly either way.Testing
Unit testing
PreferencesServiceTests: start the service, queue a write, reset the dispatchers, and assert no "Error with Preference work loop" line is logged afterwards. Red before the fix (22,723), green after.PreferencesServiceTestsand spotless pass on this branch.Manual testing
Not needed: the only cancelling caller is test-only.
Affected code checklist
Checklist
Overview
Testing
Final pass