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); 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; +}); 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)];