Skip to content

fix(offline): await reset completion and preserve concurrent packs - #4357

Open
RobJacobson wants to merge 2 commits into
rnmapbox:mainfrom
RobJacobson:codex/issue-3076-reset-database-pr
Open

RobJacobson wants to merge 2 commits into
rnmapbox:mainfrom
RobJacobson:codex/issue-3076-reset-database-pr

Conversation

@RobJacobson

@RobJacobson RobJacobson commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

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/maps 10.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.

Platform Implementation Reset sequences passing Packs immediately / after 900 ms Reset duration
iOS 27.0 Base with delays 0/10 2 / 2 0-2 ms
iOS 27.0 Fix with delays 10/10 0 / 0 843-853 ms
iOS 27.0 Final code without delays 10/10 0 / 0 2-9 ms
Android API 36 Base with delays 0/10 2 / 0 1-21 ms
Android API 36 Fix with delays 10/10 0 / 0 812-880 ms
Android API 36 Final code without delays 10/10 0 / 0 6-22 ms

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.js adds 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.
  • Library and example tsc --noEmit checks passed after stale ignored lib/ output from an unrelated branch was removed.
  • ESLint passed without warnings in the changed files. The full repository run had zero errors and 266 existing warnings.
  • The iOS simulator build and Android debug build passed against the final native source.
  • yarn generate passed 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 2da74a0d for the before checks and 3eb312cd for the fixed checks.

Platform Before: scenarios passing Fixed: scenarios passing
iOS 27.0 0/30 30/30
Android API 37 0/30 30/30

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 generate passed 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.ts

Save the component as example/src/examples/BugReportExample.js and temporarily import it as App in example/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.

import React, { useEffect, useRef, useState } from 'react';
import { Button, NativeModules, Platform, ScrollView, Text, View } from 'react-native';
import Mapbox, { offlineManager } from '@rnmapbox/maps';
import beforeManager from '../../../src/modules/offline/offlineManagerBefore';
import config from '../utils/config';

Mapbox.setAccessToken(config.get('accessToken'));
const native = NativeModules.RNMBXOfflineModule;
const delay = (ms) => new Promise((resolve) => setTimeout(resolve, ms));
const deferred = () => { let resolve; const promise = new Promise((r) => { resolve = r; }); return { promise, resolve }; };

export default function BugReportExample() {
  const running = useRef(false);
  const [lines, setLines] = useState([]);
  const log = (line) => { console.log('[4357]', line); setLines((old) => [...old, line]); };
  const run = async () => {
    if (running.current) return;
    running.current = true;
    setLines([]);
    const report = { platform: Platform.OS, cases: [], errors: [] };
    const originalGet = native.getPacks;
    const originalDelete = native.deletePack;
    const waitFor = async (name, present) => {
      for (let i = 0; i < 200; i++) {
        const found = (await originalGet()).some((p) => JSON.parse(p.metadata).name === name);
        if (found === present) return;
        await delay(25);
      }
      throw new Error(`Native storage did not become ${present ? 'present' : 'empty'} for ${name}`);
    };
    try {
      for (const phase of ['before', 'after']) {
        for (let iteration = 0; iteration < 10 && running.current; iteration++) {
          for (const scenario of ['delete', 'list', 'newer-fails']) {
            const name = `refresh-${phase}-${Platform.OS}-${iteration}-${scenario}`;
            await offlineManager.createPack({ name, styleURL: Mapbox.StyleURL.Street,
              bounds: [[0.1, 0.1], [0, 0]], minZoom: 0, maxZoom: 0 });
            await waitFor(name, true);
            const manager = new (phase === 'before' ? beforeManager.constructor : offlineManager.constructor)();
            const gates = [deferred(), deferred()];
            const ready = [deferred(), deferred()];
            let calls = 0;
            let deletionCalls = 0;
            native.getPacks = async () => {
              const index = calls++;
              const snapshot = await originalGet();
              ready[index].resolve();
              await gates[index].promise;
              if (scenario === 'newer-fails' && index === 1) throw new Error('refresh failed');
              return snapshot;
            };
            native.deletePack = (...args) => { deletionCalls++; return originalDelete(...args); };
            const first = scenario === 'delete' ? manager.deletePack(name) : manager.getPacks();
            const second = manager.getPacks();
            const settledSecond = second.then((value) => ({ value }), (error) => ({ error: error.message }));
            await Promise.all(ready.map((r) => r.promise));
            if (scenario === 'newer-fails') {
              gates[1].resolve();
              await settledSecond;
              gates[0].resolve();
            } else {
              gates[0].resolve();
              await delay(100);
              gates[1].resolve();
            }
            const firstResult = await first;
            const secondResult = await settledSecond;
            native.getPacks = originalGet;
            native.deletePack = originalDelete;
            if (scenario === 'delete' && deletionCalls > 0) await waitFor(name, false);
            const remains = (await originalGet()).some((p) => JSON.parse(p.metadata).name === name);
            const result = { phase, iteration, scenario, deletionCalls, remains,
              firstContainsPack: Array.isArray(firstResult) && firstResult.some((p) => p.name === name),
              secondError: secondResult.error };
            result.pass = scenario === 'delete' ? deletionCalls === 1 && !remains
              : result.firstContainsPack && (scenario !== 'newer-fails' || secondResult.error === 'refresh failed');
            report.cases.push(result);
            log(`${phase} ${iteration + 1}/10 ${scenario}: ${result.pass ? 'PASS' : 'FAIL'} calls=${deletionCalls} stored=${remains}`);
            if (remains) { await originalDelete(name); await waitFor(name, false); }
            delete offlineManager._offlinePacks[name];
            offlineManager._cacheRevision++;
          }
        }
      }
      report.complete = running.current && report.cases.length === 60;
      log(`COMPLETE ${report.cases.length} cases; fixed failures=${report.cases.filter((c) => c.phase === 'after' && !c.pass).length}`);
    } catch (error) {
      report.errors.push(String(error.message));
      log(`ERROR ${error.message}`);
    } finally {
      native.getPacks = originalGet;
      native.deletePack = originalDelete;
      running.current = false;
      const host = Platform.OS === 'android' ? '10.0.2.2' : '127.0.0.1';
      await fetch(`http://${host}:8813/results`, { method: 'POST',
        headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(report) }).catch((error) => log(error.message));
    }
  };
  useEffect(() => { run(); return () => { running.current = false; }; }, []);
  return <View style={{ flex: 1, padding: 24, paddingTop: 70, backgroundColor: 'white' }}>
    <Text style={{ fontSize: 22 }}>Concurrent native refreshes: {Platform.OS}</Text>
    <Button title="Start" onPress={run} /><Button title="Stop" onPress={() => { running.current = false; }} />
    <ScrollView>{lines.map((line, index) => <Text key={index}>{line}</Text>)}</ScrollView>
  </View>;
}

Checklist

  • I've read CONTRIBUTING.md.
  • I ran yarn generate in the root folder; generated files are unchanged.
  • I have tested the fix on the /example app.
    • In V11 mode/iOS
    • In New Architecture mode/iOS
    • In V11 mode/Android
    • In New Architecture mode/Android
  • I added/updated a sample (not applicable to this bug fix; the temporary reproducer is below).

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.js with the component below and import it as App in example/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
import React, { useEffect, useRef, useState } from 'react';
import { Button, NativeModules, Platform, ScrollView, Text, View } from 'react-native';
import Mapbox, { offlineManager } from '@rnmapbox/maps';
import config from '../utils/config';

const phase = 'after';
const iterationCount = 10;
Mapbox.setAccessToken(config.get('accessToken'));
const delay = (ms) => new Promise((resolve) => setTimeout(resolve, ms));
const native = NativeModules.RNMBXOfflineModule;
const reportURL = Platform.OS === 'android'
  ? 'http://10.0.2.2:8766/results' : 'http://127.0.0.1:8766/results';

export default function BugReportExample() {
  const running = useRef(false);
  const [lines, setLines] = useState([]);
  const log = (message) => {
    console.log('[3076]', message);
    setLines((previous) => [...previous, message]);
  };
  const run = async () => {
    if (running.current) return;
    running.current = true;
    setLines([]);
    const results = { phase, platform: Platform.OS, iterations: [], failures: [] };
    const create = async (name, styleURL = Mapbox.StyleURL.Street) => {
      await offlineManager.createPack({ name, styleURL,
        bounds: [[0.1, 0.1], [0, 0]], minZoom: 0, maxZoom: 0 });
      for (let attempt = 0; attempt < 100; attempt++) {
        try { await native.getPackStatus(name); return; } catch (_) { await delay(100); }
      }
      throw new Error(`Native pack did not appear: ${name}`);
    };
    try {
      log('Starting native reset checks');
      for (let iteration = 0; iteration < iterationCount && running.current; iteration++) {
        await offlineManager.resetDatabase();
        await delay(900);
        await create(`issue3076-${Platform.OS}-${iteration}-a`);
        await create(`issue3076-${Platform.OS}-${iteration}-b`, Mapbox.StyleURL.Dark);
        const before = (await offlineManager.getPacks()).length;
        const started = Date.now();
        await offlineManager.resetDatabase();
        const elapsed = Date.now() - started;
        const immediate = (await offlineManager.getPacks()).length;
        await delay(900);
        const delayed = (await offlineManager.getPacks()).length;
        results.iterations.push({ iteration, before, immediate, delayed, elapsed });
        if (before !== 2 || immediate !== 0 || delayed !== 0) results.failures.push(`iteration ${iteration}`);
        log(`${iteration + 1}/10: before=${before} immediate=${immediate} delayed=${delayed} reset=${elapsed}ms`);
      }
      await offlineManager.resetDatabase();
      await delay(900);
      const name = `issue3076-${Platform.OS}-error`;
      await create(name);
      const reset = offlineManager.resetDatabase();
      const deletion = delay(100).then(() => native.deletePack(name));
      try { await reset; results.removalErrorRejected = false; }
      catch (error) { results.removalErrorRejected = true; results.removalError = String(error.message); }
      await deletion;
      await delay(900);
      if (!results.removalErrorRejected) results.failures.push('removal error was not rejected');
      log(`Removal error rejected: ${results.removalErrorRejected}`);
      await offlineManager.resetDatabase();
      results.emptyResetPacks = (await offlineManager.getPacks()).length;
      if (results.emptyResetPacks !== 0) results.failures.push('empty reset');
      const originalGetPacks = native.getPacks;
      const originalDeletePack = native.deletePack;
      let release;
      let snapshotReady;
      const heldResponse = new Promise((resolve) => { release = resolve; });
      const ready = new Promise((resolve) => { snapshotReady = resolve; });
      let holdNext = true;
      let deleteCalls = 0;
      try {
        native.getPacks = async () => {
          const snapshot = await originalGetPacks();
          if (holdNext) { holdNext = false; snapshotReady(); await heldResponse; }
          return snapshot;
        };
        native.deletePack = (...args) => { deleteCalls++; return originalDeletePack(...args); };
        const refresh = offlineManager.getPacks();
        await ready;
        const raceName = `issue3076-${Platform.OS}-cache-race`;
        await create(raceName);
        release();
        await refresh;
        await offlineManager.deletePack(raceName);
        await delay(200);
        const remaining = (await originalGetPacks()).filter((pack) => JSON.parse(pack.metadata).name === raceName).length;
        results.cacheRace = { deleteCalls, remaining };
        if (deleteCalls !== 1 || remaining !== 0) results.failures.push('cache race skipped deletion');
        log(`Cache race: delete calls=${deleteCalls} remaining=${remaining}`);
      } finally {
        release();
        native.getPacks = originalGetPacks;
        native.deletePack = originalDeletePack;
      }
      results.complete = running.current && results.iterations.length === iterationCount;
      log(`COMPLETE: ${results.failures.length} failures`);
    } catch (error) {
      results.failures.push(String(error.message));
      results.complete = false;
      log(`ERROR: ${error.message}`);
    } finally {
      running.current = false;
      await fetch(reportURL, { method: 'POST', headers: { 'Content-Type': 'application/json' },
        body: JSON.stringify(results) }).catch((error) => log(`Report: ${error.message}`));
    }
  };
  useEffect(() => { run(); return () => { running.current = false; }; }, []);
  return <View style={{ flex: 1, padding: 24, paddingTop: 70, backgroundColor: 'white' }}>
    <Text style={{ fontSize: 22 }}>Issue 3076 — {Platform.OS} — {phase}</Text>
    <Button title="Start" onPress={run} />
    <Button title="Stop" onPress={() => { running.current = false; }} />
    <ScrollView>{lines.map((line, index) => <Text key={index} style={{ marginTop: 8 }}>{line}</Text>)}</ScrollView>
  </View>;
}
Temporary delay patch for the base revision

Apply to base 972edbf0. Set phase to before in the component. Keep this patch out of the submitted changes.

--- a/ios/RNMBX/Offline/RNMBXOfflineModule.swift
+++ b/ios/RNMBX/Offline/RNMBXOfflineModule.swift
@@ -305,7 +305,9 @@
       switch result {
       case .success(let regions):
         regions.forEach { region in
-          self.tileStore.removeTileRegion(forId: region.id)
+          DispatchQueue.global().asyncAfter(deadline: .now() + 0.4) {
+            self.tileStore.removeTileRegion(forId: region.id)
+          }
         }
         self.offlineManager.allStylePacks { result in
           switch result {
@@ -313,7 +315,9 @@
             packs.forEach { pack in
               if let url = logged("RNMBXOfflineModule.resetDatabase invalid styleURI",fn: { return URL(string: pack.styleURI) }),
                  let styleUri = logged("RNMBXOfflineModule.resetDatabase invalid styleURI2", fn: { return StyleURI(url: url) }) {
-                self.offlineManager.removeStylePack(for: styleUri)
+                DispatchQueue.global().asyncAfter(deadline: .now() + 0.4) {
+                  self.offlineManager.removeStylePack(for: styleUri)
+                }
               }
             }
             resolve(nil)
--- a/android/src/main/java/com/rnmapbox/rnmbx/modules/RNMBXOfflineModule.kt
+++ b/android/src/main/java/com/rnmapbox/rnmbx/modules/RNMBXOfflineModule.kt
@@ -537,13 +537,17 @@
         tileStore.getAllTileRegions { expected ->
             expected.value?.also { tileRegions ->
                 tileRegions.forEach { tileRegion ->
-                    tileStore.removeTileRegion(tileRegion.id);
+                    android.os.Handler(android.os.Looper.getMainLooper()).postDelayed({
+                        tileStore.removeTileRegion(tileRegion.id)
+                    }, 400)
                 }
 
                 offlineManager.getAllStylePacks { expected ->
                     expected.value?.also { stylePacks ->
                         stylePacks.forEach { stylePack ->
-                            offlineManager.removeStylePack(stylePack.styleURI)
+                            android.os.Handler(android.os.Looper.getMainLooper()).postDelayed({
+                                offlineManager.removeStylePack(stylePack.styleURI)
+                            }, 400)
                         }
                         promise.resolve(null)
                     } ?: run {
Temporary delay patch for the fixed revision

Apply to 2da74a0d. Set phase to after in the component. Keep this patch out of the submitted changes.

--- a/ios/RNMBX/Offline/RNMBXOfflineModule.swift
+++ b/ios/RNMBX/Offline/RNMBXOfflineModule.swift
@@ -318,11 +318,13 @@
 
         regions.forEach { region in
           removals.enter()
-          self.tileStore.removeRegion(forId: region.id) { result in
-            if case .failure(let error) = result {
-              recordRemovalError(error)
+          DispatchQueue.main.asyncAfter(deadline: .now() + 0.4) {
+            self.tileStore.removeRegion(forId: region.id) { result in
+              if case .failure(let error) = result {
+                recordRemovalError(error)
+              }
+              removals.leave()
             }
-            removals.leave()
           }
         }
 
@@ -344,11 +346,13 @@
                 }
 
                 stylePackRemovals.enter()
-                self.offlineManager.removeStylePack(for: styleUri) { result in
-                  if case .failure(let error) = result {
-                    recordRemovalError(error)
+                DispatchQueue.main.asyncAfter(deadline: .now() + 0.4) {
+                  self.offlineManager.removeStylePack(for: styleUri) { result in
+                    if case .failure(let error) = result {
+                      recordRemovalError(error)
+                    }
+                    stylePackRemovals.leave()
                   }
-                  stylePackRemovals.leave()
                 }
               }
 
--- a/android/src/main/java/com/rnmapbox/rnmbx/modules/RNMBXOfflineModule.kt
+++ b/android/src/main/java/com/rnmapbox/rnmbx/modules/RNMBXOfflineModule.kt
@@ -559,16 +559,18 @@
                     val remaining = java.util.concurrent.atomic.AtomicInteger(stylePacks.size)
                     val failed = java.util.concurrent.atomic.AtomicBoolean(false)
                     stylePacks.forEach { stylePack ->
-                        offlineManager.removeStylePack(stylePack.styleURI, object : StylePackCallback {
-                            override fun run(result: Expected<StylePackError, StylePack>) {
-                                val removed = result.value
-                                if (removed == null && failed.compareAndSet(false, true)) {
-                                    promise.reject(Error(result.error?.message ?: "n/a"))
-                                } else if (remaining.decrementAndGet() == 0 && !failed.get()) {
-                                    promise.resolve(null)
-                                }
-                            }
-                        })
+                        android.os.Handler(android.os.Looper.getMainLooper()).postDelayed({
+                          offlineManager.removeStylePack(stylePack.styleURI, object : StylePackCallback {
+                              override fun run(result: Expected<StylePackError, StylePack>) {
+                                  val removed = result.value
+                                  if (removed == null && failed.compareAndSet(false, true)) {
+                                      promise.reject(Error(result.error?.message ?: "n/a"))
+                                  } else if (remaining.decrementAndGet() == 0 && !failed.get()) {
+                                      promise.resolve(null)
+                                  }
+                              }
+                          })
+                        }, 400)
                     }
                 }
             }
@@ -581,16 +583,18 @@
             val remaining = java.util.concurrent.atomic.AtomicInteger(tileRegions.size)
             val failed = java.util.concurrent.atomic.AtomicBoolean(false)
             tileRegions.forEach { tileRegion ->
-                tileStore.removeTileRegion(tileRegion.id, object : TileRegionCallback {
-                    override fun run(result: Expected<TileRegionError, TileRegion>) {
-                        val removed = result.value
-                        if (removed == null && failed.compareAndSet(false, true)) {
-                            promise.reject(Error(result.error?.message ?: "n/a"))
-                        } else if (remaining.decrementAndGet() == 0 && !failed.get()) {
-                            removeStylePacks()
-                        }
-                    }
-                })
+                android.os.Handler(android.os.Looper.getMainLooper()).postDelayed({
+                  tileStore.removeTileRegion(tileRegion.id, object : TileRegionCallback {
+                      override fun run(result: Expected<TileRegionError, TileRegion>) {
+                          val removed = result.value
+                          if (removed == null && failed.compareAndSet(false, true)) {
+                              promise.reject(Error(result.error?.message ?: "n/a"))
+                          } else if (remaining.decrementAndGet() == 0 && !failed.get()) {
+                              removeStylePacks()
+                          }
+                      }
+                  })
+                }, 400)
             }
         }
 

…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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: offlineManager.resetDatabase not clearing pack list

1 participant