Repository navigation
fix(offline): await reset completion and preserve concurrent packs - #4357
Open
RobJacobson wants to merge 2 commits into
Open
RobJacobson wants to merge 2 commits into
RobJacobson wants to merge 2 commits into
Conversation
…rnmapbox#3076) - Preserve current and legacy pack mutations when older refresh responses arrive. - Rebuild pack lists from native storage and reject obsolete snapshots after reset or newer refreshes. - Wait for tile-region and style-pack removals on both platforms and propagate failures. - Continue iOS style-pack reset work on the main queue to prevent cold reset stalls. - Add 22 reset and concurrent-refresh regression cases. - Verify delayed and uninstrumented resets and cache races in both example apps.
- Discard older snapshots only after a newer refresh updates the cache - Cover cold-start deletion, overlapping reads, and failed newer refreshes in both managers - Verify six regressions, 194 unit tests, and native before/after checks on iOS and Android
This branch has not been deployed
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
Fixes #3076.
resetDatabase()waits for tile-region and style-pack removal callbacks before resolving. Both managers replace the pack list from native storage and discard responses made obsolete by a mutation or an applied newer refresh. This prevents a delayed list response from removing a newly created pack or restoring a deleted pack.The iOS style stage continues on the main queue. A cold-reset check stalled before the style callback when that stage ran on a global queue.
Validation
Local native verification used commit 2da74a0d and base 972edbf0 on October 13, 2026. The example app used React Native 0.84.1, Fabric, and Mapbox Maps SDK 11.23.1. The issue's reported
@rnmapbox/maps10.0.15 environment was not rerun.The stress checks temporarily delayed each native removal invocation by 400 ms. They also deleted a region during reset to provoke a removal error. All delays, logging, and example-app setup changes were removed from this branch before the final runs.
The delayed base runs resolved successfully despite a removal error. The delayed fixed runs rejected that error on both platforms. Empty resets also passed.
The cache-race check held an actual native list response until creation completed, then called
deletePack(). The initial proposed cache replacement made zero native deletion calls and left the region stored on Android. The final implementation made one deletion call and left no region on both platforms.offlineManagerReset.test.jsadds 22 cases covering reset completion, errors, empty storage, stale responses around reset, concurrent creation and deletion, and out-of-order refreshes. Ten of those cases failed against the initial proposed cache replacement.node node_modules/jest/bin/jest.js --runInBand --watchman=false: 194 tests passed across 26 suites, rerun at the PR head on October 14, 2026.tsc --noEmitchecks passed after stale ignoredlib/output from an unrelated branch was removed.yarn generatepassed at the PR head on October 14, 2026, without changing generated files.Cold-start concurrency verification
Both managers accept a successful older response while a newer refresh is pending or has failed. A newer response supersedes older snapshots after it updates the cache; mutations still invalidate snapshots.
At commit 3eb312cd on October 14, 2026, six additional regression cases passed in offlineManagerReset.test.js. All six failed against
2da74a0d. They cover deletion during initial loading, two overlapping initial reads, and a newer initial read failing, for both managers.The installed example apps ran ten repetitions of each scenario with real native storage. The reproducer holds actual native list responses to force their order and injects the failed newer response in JavaScript. Both runs use the same native implementation, with JavaScript from
2da74a0dfor the before checks and3eb312cdfor the fixed checks.Every before deletion made zero native deletion calls and left its pack stored. Every fixed deletion made one native deletion call and left no pack. The complete runs had no unexpected errors. Library and example type checks passed, changed-file ESLint passed with zero warnings, and
yarn generatepassed without generated changes. The full lint run had zero errors and the same 266 existing warnings.Runnable cold-start concurrency reproducer
Use the example-app setup described below with this component instead. Create a temporary baseline module in the repository root:
git show 2da74a0d:src/modules/offline/offlineManager.ts > src/modules/offline/offlineManagerBefore.tsSave the component as
example/src/examples/BugReportExample.jsand temporarily import it asAppinexample/index.js. It creates and removes its own temporary packs. The local report collector at port 8813 is optional; the screen shows each result. Remove the baseline module and restore the example files afterward. No native rebuild is needed for this JavaScript-only follow-up.Checklist
CONTRIBUTING.md.yarn generatein the root folder; generated files are unchanged./exampleapp.Screenshot OR Video
The table above records the pack-list values and completion timings. This change has no visual behavior.
Component to reproduce the issue
Use the existing example-app token configuration. Replace
example/src/examples/BugReportExample.jswith the component below and import it asAppinexample/index.js. It starts automatically and provides Start/Stop controls. The final HTTP report upload is optional; the screen and console show the results without a collector.For the delayed checks, rebuild native code after applying the corresponding temporary patch below. The induced-removal-error check depends on those delays. For an uninstrumented run, omit that error-injection block; the reset sequences and held-response cache check remain applicable.
The iOS runs used temporary scene/deployment setup adjustments for the installed React Native 0.84.1 and iOS 27 runtime. Those example-app adjustments are outside this PR.
Runnable example component
Temporary delay patch for the base revision
Apply to base
972edbf0. Setphasetobeforein the component. Keep this patch out of the submitted changes.Temporary delay patch for the fixed revision
Apply to
2da74a0d. Setphasetoafterin the component. Keep this patch out of the submitted changes.