From dea844eb32ce645923f8c48e412d89a774e471d6 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Sun, 6 Sep 2026 23:22:47 +0900 Subject: [PATCH 1/3] test(e2e): reproduce a reload racing an in-flight download ## Summary - Add a standalone E2E scenario that taps "Check for updates" and then "Restart app" while the update archive is still downloading. - Throttle archive responses in the mock server when `E2E_SLOW_DOWNLOAD_MS` is set, so the reload lands in the middle of a download instead of after it. Without this the archive arrives over localhost in one chunk and the scenario proves nothing. - Run it from `e2e/repro-reload-race.ts`, which releases one optional update and runs that one flow rather than the whole suite. `--build` rebuilds the app for a native change, and `--skip-release` reuses the update already being served. - Report what the device recorded either way: the matching iOS crash report, or the logcat lines around a crash on Android. ## Verification On iOS (RN0840, new architecture) the scenario crashes the app in two runs out of two: EXC_CRASH (SIGABRT) -[CodePushDownloadHandler connection:didReceiveData:] -[CodePush downloadUpdate:...]_block_invoke -[CodePush dispatchDownloadProgressEvent] -[NativeCodePushSpecBase emitOnDownloadProgress:] The download that outlives the reload reaches the event emitter of a runtime that has already been torn down. Android is unverified: no emulator was available. --- .../01-restart-during-download.yaml | 42 ++++ e2e/mock-server/server.ts | 39 +++ e2e/repro-reload-race.ts | 230 ++++++++++++++++++ 3 files changed, 311 insertions(+) create mode 100644 e2e/flows-reload-race/01-restart-during-download.yaml create mode 100644 e2e/repro-reload-race.ts diff --git a/e2e/flows-reload-race/01-restart-during-download.yaml b/e2e/flows-reload-race/01-restart-during-download.yaml new file mode 100644 index 00000000..4f7988f8 --- /dev/null +++ b/e2e/flows-reload-race/01-restart-during-download.yaml @@ -0,0 +1,42 @@ +appId: ${APP_ID} +--- +# A reload fired while an update is still downloading leaves the finishing download talking +# to a runtime that is gone: the progress callback of the generation that started the +# download reaches the JS side after that generation was torn down. +# +# The download is throttled by the mock server for this scenario, so the reload lands while +# it is still running. +- launchApp: + clearState: true +- runFlow: + when: + platform: Android + file: ../flows/shared/android-dismiss-overlays.yaml +- runFlow: + when: + platform: iOS + file: ../flows/shared/ios-dismiss-overlays.yaml +- assertVisible: "React Native.*" + +# The release is optional, so it installs on the next restart rather than restarting by +# itself, and the app stays on screen for the whole download. +- tapOn: "Check for updates" + +# The download has to still be running when the restart is tapped. Without this the +# scenario can pass having proved nothing. +- assertVisible: "Result: DOWNLOADING_PACKAGE" + +- tapOn: "Restart app" + +- waitForAnimationToEnd: + timeout: 25000 +- tapOn: + text: "(?i)^wait$" + optional: true + +# Survived the reload: the process is still the app and it still answers taps. +- assertVisible: "React Native.*" +- tapOn: "Get update metadata" +- waitForAnimationToEnd: + timeout: 10000 +- assertVisible: "METADATA_.*" diff --git a/e2e/mock-server/server.ts b/e2e/mock-server/server.ts index fca4d3d3..d4fc33a4 100644 --- a/e2e/mock-server/server.ts +++ b/e2e/mock-server/server.ts @@ -1,4 +1,6 @@ import express from "express"; +import fs from "fs"; +import path from "path"; import { getMockDataDir, getMockServerPort } from "../config"; import type { Server } from "http"; @@ -74,6 +76,43 @@ export function startMockServer(platform: Platform): Promise { res.status(204).end(); }); + // A reload racing an in-flight download only says something while the download is + // still running, and a Metro bundle arrives over localhost in a single chunk. When a + // scenario asks for it, archives are served in slices spread over that many + // milliseconds so the reload lands in the middle of one. + const slowDownloadMs = Number(process.env.E2E_SLOW_DOWNLOAD_MS ?? 0); + if (slowDownloadMs > 0) { + app.use((req: express.Request, res: express.Response, next: express.NextFunction) => { + // Archives are served under /bundles with a hash for a name, so what is throttled + // follows from where the file is rather than from what it is called. + if (!req.path.startsWith("/bundles/")) { + return next(); + } + const filePath = path.join(dataDir, path.normalize(req.path)); + if (!filePath.startsWith(dataDir) || !fs.existsSync(filePath)) { + return next(); + } + + const body = fs.readFileSync(filePath); + const sliceCount = 20; + const sliceSize = Math.ceil(body.length / sliceCount); + res.setHeader("Content-Type", "application/zip"); + res.setHeader("Content-Length", String(body.length)); + + let offset = 0; + const writeNextSlice = () => { + if (offset >= body.length) { + res.end(); + return; + } + res.write(body.subarray(offset, offset + sliceSize)); + offset += sliceSize; + setTimeout(writeNextSlice, slowDownloadMs / sliceCount); + }; + writeNextSlice(); + }); + } + app.use(express.static(dataDir)); app.use((_req: express.Request, res: express.Response) => { diff --git a/e2e/repro-reload-race.ts b/e2e/repro-reload-race.ts new file mode 100644 index 00000000..cfbe3175 --- /dev/null +++ b/e2e/repro-reload-race.ts @@ -0,0 +1,230 @@ +/** + * Standalone reproduction for the reload-during-download race. + * + * It runs one scenario and nothing else: release one optional update, serve its archive + * slowly, and reload the app in the middle of the download. The full runner's other + * phases say nothing about this race, so none of them are run here. + * + * The app must already be built and installed, or --build has to be passed: native + * changes only reach the device through a build. + * + * npx tsx e2e/repro-reload-race.ts --app RN0840 --platform ios + */ +import { spawn } from "child_process"; +import fs from "fs"; +import os from "os"; +import path from "path"; +import { Command } from "commander"; +import { getAppPath, getMockDataDir } from "./config"; +import { buildApp } from "./helpers/build-app"; +import { prepareConfig } from "./helpers/prepare-config"; +import { prepareBundle } from "./helpers/prepare-bundle"; +import { startMockServer, stopMockServer } from "./mock-server/server"; + +type Platform = "ios" | "android"; + +const FLOW_DIR = path.resolve(__dirname, "flows-reload-race"); +const DEFAULT_SLOW_DOWNLOAD_MS = 12000; + +const program = new Command() + .requiredOption("--app ", "example app directory name") + .requiredOption("--platform ", "ios or android") + .option("--slow-download-ms ", "how long one archive takes to arrive", String(DEFAULT_SLOW_DOWNLOAD_MS)) + .option("--skip-release", "reuse the update already in the mock data directory") + .option("--build", "rebuild and install the app first, for a native change"); + +async function main(): Promise { + const options = program.parse(process.argv).opts<{ + app: string; + platform: string; + slowDownloadMs: string; + skipRelease?: boolean; + build?: boolean; + }>(); + + const platform = options.platform as Platform; + if (platform !== "ios" && platform !== "android") { + throw new Error(`Invalid --platform: ${options.platform}`); + } + + const appPath = getAppPath(options.app); + if (!fs.existsSync(appPath)) { + throw new Error(`Example app not found: ${appPath}`); + } + + process.env.E2E_SLOW_DOWNLOAD_MS = options.slowDownloadMs; + + const appId = readAppId(appPath, platform); + const releaseIdentifier = readReleaseIdentifier(appPath); + const startedAt = Date.now(); + + prepareConfig(appPath, platform); + + if (options.build || !options.skipRelease) { + // The copy of the library inside the app is what a build compiles and what releases + // the update, so it has to be the one this checkout holds. + console.log("\n=== [sync-local-library] ==="); + await run("npm", ["run", "sync-local-library", "--prefix", appPath]); + } + + if (options.build) { + console.log("\n=== [build] ==="); + await buildApp(appPath, platform); + } + + if (!options.skipRelease) { + console.log("\n=== [prepare-bundle] ==="); + const dataDir = getMockDataDir(platform); + fs.rmSync(dataDir, { recursive: true, force: true }); + fs.mkdirSync(dataDir, { recursive: true }); + // Optional, so the update waits for a restart instead of restarting by itself: the + // reload this scenario races has to be the one the flow taps. + await prepareBundle(appPath, platform, releaseIdentifier, undefined, { mandatory: false }); + } + + console.log(`\n=== [start-mock-server] (archives spread over ${options.slowDownloadMs}ms) ===`); + await startMockServer(platform); + + if (platform === "android") { + await run("adb", ["reverse", "tcp:18082", "tcp:18082"]).catch(() => undefined); + await run("adb", ["logcat", "-c"]); + } + + try { + console.log("\n=== [run-maestro: restart during download] ==="); + await runMaestro(FLOW_DIR, platform, appId); + console.log("\n=== flow passed: the app survived the reload ==="); + } catch (error) { + console.error(`\n=== flow failed: ${(error as Error).message} ===`); + process.exitCode = 1; + } finally { + await stopMockServer(platform); + await reportCrashEvidence(platform, options.app, appId, startedAt); + } +} + +/** + * What the device recorded while the flow ran. + * + * A failing assertion says the app went away but not why, and the app can also survive the + * race while logging the very error this is looking for - so the evidence is printed + * whether the flow passed or failed. + */ +async function reportCrashEvidence( + platform: Platform, + appName: string, + appId: string, + startedAt: number, +): Promise { + console.log("\n=== [device evidence] ==="); + + if (platform === "android") { + const logcat = await capture("adb", ["logcat", "-d", "-t", "4000"]); + const lines = logcat + .split("\n") + .filter((line) => /FATAL EXCEPTION|AndroidRuntime|IllegalStateException|CodePush/.test(line)); + console.log(lines.length ? lines.slice(-60).join("\n") : "logcat has nothing about a crash or CodePush"); + return; + } + + const reportDir = path.join(os.homedir(), "Library/Logs/DiagnosticReports"); + if (!fs.existsSync(reportDir)) { + console.log(`no crash report directory at ${reportDir}`); + return; + } + + const reports = fs + .readdirSync(reportDir) + .filter((name) => name.includes(appName) || name.includes(appId)) + .map((name) => path.join(reportDir, name)) + .filter((file) => fs.statSync(file).mtimeMs >= startedAt); + + if (!reports.length) { + console.log("no crash report was written while the flow ran"); + return; + } + + // A crash report is mostly threads that were idle. What is worth printing is the kind of + // crash and the frames that name this library, which is what says the finishing download + // is what reached into the runtime that had gone. + for (const report of reports) { + console.log(`\n--- ${report} ---`); + const content = fs.readFileSync(report, "utf8"); + const exception = content.match(/"exception"\s*:\s*\{[^}]*\}/)?.[0]; + if (exception) { + console.log(exception); + } + const frames = [...content.matchAll(/"symbol"\s*:\s*"([^"]*(?:CodePush|RNCodePushSpec)[^"]*)"/g)] + .map((match) => match[1]); + console.log(frames.length ? [...new Set(frames)].join("\n") : "no frame names this library"); + } +} + +function runMaestro(flowsDir: string, platform: Platform, appId: string): Promise { + if (platform === "ios") { + return run("maestro", ["test", "--platform", "ios", "-e", `APP_ID=${appId}`, flowsDir]); + } + return run("maestro-runner", [ + "--platform", "android", + "test", + "--output", path.resolve(__dirname, "reports"), + "--env", `APP_ID=${appId}`, + flowsDir, + ]); +} + +function run(command: string, args: string[]): Promise { + console.log(`[command] ${command} ${args.join(" ")}`); + return new Promise((resolve, reject) => { + const child = spawn(command, args, { stdio: "inherit" }); + child.on("error", reject); + child.on("close", (code) => { + if (code === 0) resolve(); + else reject(new Error(`${command} exited with ${code}`)); + }); + }); +} + +function capture(command: string, args: string[]): Promise { + return new Promise((resolve) => { + const child = spawn(command, args); + let output = ""; + child.stdout.on("data", (chunk) => (output += chunk)); + child.on("error", () => resolve("")); + child.on("close", () => resolve(output)); + }); +} + +function readAppId(appPath: string, platform: Platform): string { + const appJson = JSON.parse(fs.readFileSync(path.join(appPath, "app.json"), "utf8")) as { + name?: string; + expo?: { ios?: { bundleIdentifier?: string }; android?: { package?: string } }; + }; + + if (platform === "ios") { + const expoId = appJson.expo?.ios?.bundleIdentifier; + if (expoId) return expoId; + if (!appJson.name) throw new Error("Could not find iOS app identifier in app.json"); + return `com.${appJson.name.toLowerCase().replace(/[^a-z0-9]+/g, "")}`; + } + + const expoPackage = appJson.expo?.android?.package; + if (expoPackage) return expoPackage; + + const gradle = fs.readFileSync(path.join(appPath, "android/app/build.gradle"), "utf8"); + const match = gradle.match(/applicationId\s+["']([^"']+)["']/) ?? gradle.match(/namespace\s+["']([^"']+)["']/); + if (!match) throw new Error("Could not find Android app identifier"); + return match[1]; +} + +function readReleaseIdentifier(appPath: string): string { + const content = fs.readFileSync(path.join(appPath, "App.tsx"), "utf8"); + const match = content.match(/const IDENTIFIER = ['"]([^'"]+)['"]/); + if (!match) throw new Error("Could not find CodePush IDENTIFIER in App.tsx"); + return match[1]; +} + +void main().catch((error) => { + console.error(error); + process.exitCode = 1; +}); From 74915cdc5ebc00340d4bc92944312d9f20d36e55 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Sun, 6 Sep 2026 23:22:55 +0900 Subject: [PATCH 2/3] fix(ios): ignore work from a React Native instance that was replaced ## Summary - Adopt `RCTInvalidating`, so React Native tells the module when the instance it belongs to is being replaced. - Track that in `_generationAlive` and return early from `dispatchDownloadProgressEvent`, `loadBundle` and `restartAppInternal:` once it is false. - Invalidate the suspend timer and remove the resume observers at the same point, so neither can restart the instance that replaced this one. A reload starts the next instance before the one being replaced has finished being torn down. A download in flight keeps this module alive across that boundary, and its progress callback then reaches an event emitter whose runtime is gone. The suspend timer and the resume notifications reach the module the same way, and the restart they ask for belongs to an instance that is no longer on screen. ## Verification - `npx tsx e2e/repro-reload-race.ts --app RN0840 --platform ios`: the scenario that crashed in two runs out of two now passes, with no crash report written. - `npm run jest`: 11 suites, 102 tests. Android carries the same exposure through its own progress path and is not covered here. --- ios/CodePush/CodePush.mm | 62 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/ios/CodePush/CodePush.mm b/ios/CodePush/CodePush.mm index aa347bf7..80887c09 100644 --- a/ios/CodePush/CodePush.mm +++ b/ios/CodePush/CodePush.mm @@ -1,6 +1,7 @@ #import #import #import +#import #import #import #import @@ -8,7 +9,9 @@ #import "CodePush.h" -@interface CodePush () + +@interface CodePush () _generationAlive; } RCT_EXPORT_MODULE() @@ -307,7 +321,42 @@ - (void)dealloc [[NSNotificationCenter defaultCenter] removeObserver:self]; } +/* + * Called when the React Native instance this module belongs to is being replaced. + * + * A download in flight keeps this module alive past that point, and the suspend timer and + * the resume notifications keep it reachable too. None of them may act afterwards: the + * runtime they would report to is gone, and the restart they would trigger belongs to a + * generation that is no longer on screen. + */ +- (void)invalidate +{ + _generationAlive.store(false); + + [_appSuspendTimer invalidate]; + _appSuspendTimer = nil; + _hasResumeListener = NO; + [[NSNotificationCenter defaultCenter] removeObserver:self]; + +#ifndef RCT_NEW_ARCH_ENABLED + [super invalidate]; +#endif +} + +/* Whether this module still belongs to the React Native instance that is running. */ +- (BOOL)isGenerationAlive +{ + return _generationAlive.load(); +} + - (void)dispatchDownloadProgressEvent { + // The download that is reporting here can be one this module started before the app was + // reloaded, in which case the event emitter it would reach belongs to a runtime that has + // been torn down. + if (![self isGenerationAlive]) { + return; + } + // Notify the script-side about the progress NSDictionary *progress = @{ @"totalBytes" : [NSNumber numberWithLongLong:_latestExpectedContentLength], @@ -375,6 +424,7 @@ - (instancetype)init _restartInProgress = NO; _restartQueue = [NSMutableArray arrayWithCapacity:1]; _lastProgressEventTime = 0; + _generationAlive.store(true); self = [super init]; if (self) { @@ -533,6 +583,11 @@ + (BOOL)isPendingUpdate:(NSString*)packageHash */ - (void)loadBundle { + if (![self isGenerationAlive]) { + CPLog(@"Ignoring a reload requested by a React Native instance that is no longer running."); + return; + } + @synchronized([CodePush class]) { hasInitializedUpdateAfterRestartForCurrentLoad = NO; } @@ -802,6 +857,11 @@ -(void)loadBundleOnTick:(NSTimer *)timer { - (void)restartAppInternal:(BOOL)onlyIfUpdateIsPending { + if (![self isGenerationAlive]) { + CPLog(@"Ignoring a restart requested by a React Native instance that is no longer running."); + return; + } + if (_restartInProgress) { CPLog(@"Restart request queued until the current restart is completed."); [_restartQueue addObject:@(onlyIfUpdateIsPending)]; From dc9972e9b30909d16de03f91da450f793c80b135 Mon Sep 17 00:00:00 2001 From: Floyd Kim Date: Sun, 6 Sep 2026 23:27:34 +0900 Subject: [PATCH 3/3] fix(android): ignore download progress from a module that was invalidated ## Summary - Track whether the React Native instance this module belongs to is still running in `mGenerationAlive`, cleared in `invalidate()`. - Return early from `emitDownloadProgressEvent` once it is false. Every progress event routes through there, including the ones a frame callback delivers after teardown. - Return early from `restartAppInternal` too, so a module that has been replaced cannot restart the instance that replaced it. `invalidate()` shuts the background executor down, but a frame callback already posted to `ReactChoreographer` belongs to a process-wide singleton and runs regardless. It then emits through a context that has been destroyed. ## Verification `e2e/repro-reload-race.ts --app RN0840 --platform android`, against a Pixel emulator on API 36 with the new architecture: - Without this change the app dies on the frame callback: FATAL EXCEPTION: main java.lang.RuntimeException: __next_prime overflow NativeCodePushSpec.emitOnDownloadProgress CodePushNativeModule.emitDownloadProgressEvent ...doFrame android.view.Choreographer$CallbackRecord.run This is the signature the iOS crash carried as well. - With it the scenario passes. The download the reload interrupts ends in the `InterruptedIOException` that shutting the executor down is meant to raise, which is logged and rejects its promise. It reaches the general catch rather than the invalid-update one, so no package hash is recorded as failed. - `:bravemobile_react-native-code-push:compileReleaseJavaWithJavac` --- .../codepush/react/CodePushNativeModule.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java b/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java index 79c4ec20..9bf8b0e9 100644 --- a/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java +++ b/android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java @@ -59,6 +59,15 @@ public class CodePushNativeModule extends NativeCodePushSpec { private boolean _restartInProgress = false; private ArrayList _restartQueue = new ArrayList<>(); + /** + * Whether the React Native instance this module was created for is still the one running. + * + * A reload replaces the instance while work this module started can still be in flight, + * and a frame callback posted to the process-wide choreographer outlives the executor + * this module shuts down. Read from whichever thread that work is on, so it is volatile. + */ + private volatile boolean mGenerationAlive = true; + public CodePushNativeModule(ReactApplicationContext reactContext, CodePush codePush, CodePushUpdateManager codePushUpdateManager, CodePushTelemetryManager codePushTelemetryManager, SettingsManager settingsManager) { super(reactContext); @@ -85,6 +94,7 @@ public String getName() { @Override public void invalidate() { + mGenerationAlive = false; clearLifecycleEventListener(); mBackgroundExecutor.shutdownNow(); super.invalidate(); @@ -264,6 +274,14 @@ private void executeInBackground(Runnable runnable) { } private void emitDownloadProgressEvent(DownloadProgress downloadProgress) { + // Every progress event arrives here, including the ones a frame callback delivers + // after this module was invalidated: shutting the executor down does not cancel a + // callback already posted to the choreographer, and the context it would emit + // through has been destroyed by then. + if (!mGenerationAlive) { + return; + } + if (mEventEmitterCallback != null) { emitOnDownloadProgress(downloadProgress.createWritableMap()); return; @@ -275,6 +293,13 @@ private void emitDownloadProgressEvent(DownloadProgress downloadProgress) { } private void restartAppInternal(boolean onlyIfUpdateIsPending) { + // A restart requested by an instance that has been replaced would reload the one + // that replaced it, which nobody asked for. + if (!mGenerationAlive) { + CodePushUtils.log("Ignoring a restart requested by a React Native instance that is no longer running"); + return; + } + if (this._restartInProgress) { CodePushUtils.log("Restart request queued until the current restart is completed"); this._restartQueue.add(onlyIfUpdateIsPending);