Skip to content

fix(android): stop split-bundle extraction racing itself - #108

Merged
huhuanming merged 5 commits into
mainfrom
fix/split-bundle-extract-race
Sep 16, 2026
Merged

huhuanming merged 5 commits into
mainfrom
fix/split-bundle-extract-race

Conversation

@huhuanming

@huhuanming huhuanming commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Problem

A user hit a blank tab right after upgrading the APK (6.3.0 → 6.5.0). From the attached logs, one segment failed to load and stayed failed for the rest of the process:

11:30:29 [resolveSeg] rel=segments/nm._onekeyfe.seg.hbc otaBundlePath=(empty)   ← thread A
11:30:29 [resolveSeg] rel=segments/nm._onekeyfe.seg.hbc otaBundlePath=(empty)   ← thread B
11:30:29 [extractBuiltin] extracted ... (4708936 bytes)                          ← A renamed
11:30:29 [resolveSeg] → builtin /data/.../6.5.0/segments/nm._onekeyfe.seg.hbc    ← A returned the path
11:30:29 WARN [extractBuiltin] rename failed ... .seg.hbc.tmp → .seg.hbc         ← B's rename failed
11:30:29 WARN [resolveSeg] → null (builtin extract failed)                       ← B reported "missing"
11:30:29 [SplitBundle] SEGMENT LOAD FAILED code=SPLIT_BUNDLE_NOT_FOUND

The file was on disk and complete the whole time — 21 s later the same path resolved fine.

Root cause

extractBuiltinSegmentIfNeeded had three problems that only line up under concurrency:

  1. extractSemaphore is an I/O throttle, not mutual exclusion. With MAX_CONCURRENT_EXTRACTS = 2, two threads can extract the same relativePath at once — and there are exactly two callers, the main runtime and the background runtime, resolving segments independently.
  2. The temp name was shared: "<name>.tmp" is fixed per path, so both threads open the same file with O_TRUNC and write into each other's stream. Besides the rename race, that can publish a partially zeroed HBC (which then surfaces as SPLIT_BUNDLE_SHA256_MISMATCH or a Hermes eval failure).
  3. The rename loser returned null, without checking whether the destination had just been published by the winner. The JS loader caches SPLIT_BUNDLE_NOT_FOUND as a permanent failure, so a millisecond-wide race blanked a route until restart.

It shows up specifically on the first launch after an APK replace because the install-stamp wipe empties the whole extract tree, forcing every segment to re-extract at once — widest window on the largest segment (4.7 MB here). On any other launch the exists() fast path means the race never opens.

iOS is unaffected: resolveAbsolutePath only does fileExistsAtPath against the OTA dir and NSBundle.resourcePath, with no extraction, temp file, or rename.

Change

  • Serialize per path with extractPathLocks — exactly one thread extracts; the rest wake on the exists() fast path. The lock is held outside the semaphore so a waiter never sits on a permit it isn't using, and the re-check under the lock can hand back an already-extracted file without spending one. No cycle is introduced: nothing takes a path monitor while holding a permit, and the install-stamp gate always completes before either.
  • Unique temp name per attempt (<name>.<tid>-<nanoTime>.tmp) — no two writers ever share an inode, which holds across processes too, where a static lock cannot. Because a unique temp file is never reused by a later attempt, it is reclaimed in a finally: without that, every failed extraction would orphan its partial write (multi-MB for the large segments) until the next APK replace, where the old shared name left at most one.
  • Re-check the destination when renameTo fails, comparing against the bytes just written rather than re-reading the asset, and requiring an exact match — accepting an unknown size there would have been laxer than the checks above it.
  • Decide a size mismatch under the per-path lock. The old code deleted a mismatching destination outside every gate, which could unlink a file another thread had just published and hand its writer a path that no longer exists. The cheap exists() + size fast path still short-circuits without locking; only the mismatch case falls through.

Verification

  • ./gradlew :onekeyfe_react-native-split-bundle-loader:compileDebugKotlin — BUILD SUCCESSFUL, with the new code confirmed in the emitted classes.
  • The temp-file reclaim is covered by a runnable replica of the new control flow: 5 consecutive failing extractions leave 0 orphan temp files (the pre-fix shape leaves one per attempt), and the success path still publishes the full file with nothing left behind.
  • Reviewed by a separate agent against the production logs; its findings on the temp-file leak, the unlocked delete, and the fail-open size check are folded into this commit.

Also in this PR: bundle-crypto framework slimming

@onekeyfe/react-native-bundle-crypto was 14.9 MB packed / 42.5 MB unpacked — 9x the next largest package here (1.7 MB) and ~100x the median — because the vendored Gopenpgp.xcframework carries three ~10 MB gomobile static archives, one of them a fat simulator slice holding both arm64 and x86_64. Dropping x86_64 takes it to 10.7 MB / 30.7 MB. The slice directory is renamed to match its contents and the xcframework manifest updated; the podspec vendors the whole xcframework, so nothing else referenced the old name. strip -S is not an alternative — it rejects the gomobile archives with "string table not at the end of the file".

Breaking for Intel Macs. This module no longer supports iOS simulator builds on x86_64. Apple Silicon simulator, device, and Mac Catalyst are unchanged. Confirmed with the team that nobody builds the simulator on an Intel Mac, so shipping it in a 3.0.x patch is a deliberate call rather than an oversight.

The lasting fix is to stop shipping the framework inside the npm tarball and fetch it at pod install time via an http source in the podspec; that needs a hosting decision and is tracked separately.

Release

Rebased onto main and bumped all 40 publishable packages with a changelog entry, so this PR is publishable as-is.

Already published: 3.0.138, tagged latest, all 40 packages verified against the registry origin. Note 3.0.137 is a partial release — it reached 39 packages, but npm left @onekeyfe/react-native-bundle-crypto@3.0.137 staged and never committed it, so that version is undownloadable and permanently unpublishable (409 Cannot publish over previously staged version). Both CHANGELOG entries record this.

The matching pin bump to 3.0.138 in app-monorepo (package.json × 4, yarn.lock, apps/mobile/ios/Podfile.lock) is in the linked PR below.

Related

The JS half of this fix — giving SPLIT_BUNDLE_NOT_FOUND one re-attempt instead of caching it on sight — is in OneKeyHQ/app-monorepo#13461. That one reaches the binaries already in the field, since OTA bundles run on whatever native build is installed; this PR needs a new APK.

🤖 Generated with Claude Code

huhuanming and others added 2 commits September 16, 2026 09:45
The main and background runtimes resolve the same segment
independently, and extractSemaphore is an I/O throttle rather than
mutual exclusion — so both can extract one relativePath at once. They
shared a single "<name>.tmp", which means two O_TRUNC writers on the
same inode and a renameTo that fails for whichever thread loses. That
loser reported SPLIT_BUNDLE_NOT_FOUND for a file that was on disk and
complete, and the JS loader caches NOT_FOUND as a permanent failure, so
a millisecond-wide race blanked a route for the rest of the process.

Observed on the first launch after an APK replace, where the
install-stamp wipe empties the extract tree and forces every segment to
re-extract at once — widest window on the largest segment.

- serialize extraction per path with extractPathLocks, held OUTSIDE the
  semaphore so a waiter never sits on a permit it isn't using
- give each attempt a unique temp name, so concurrent writers can never
  publish a partially zeroed HBC (holds across processes too), and
  reclaim that temp file in a finally — unlike the old shared name, a
  unique one is never reused, so a failed extraction would otherwise
  orphan its partial write until the next APK replace
- re-check the destination when renameTo fails, comparing against the
  bytes just written rather than re-reading the asset, instead of
  reporting a segment another writer just published as missing
- decide a size mismatch under the per-path lock: deleting outside it
  could unlink a file another thread had just published

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bump all 40 publishable packages to 3.0.137 and record the Android
split-bundle extraction race fix in the changelog.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@huhuanming
huhuanming force-pushed the fix/split-bundle-extract-race branch from 8520124 to 52ec9d2 Compare September 16, 2026 01:49
huhuanming and others added 2 commits September 16, 2026 10:15
…ramework

The published tarball was 14.9 MB packed / 42.5 MB unpacked — 9x the
next largest package in this repo (1.7 MB) and ~100x the median —
because the vendored GopenPGP xcframework carries three ~10 MB gomobile
static archives, one of them a fat simulator slice holding both arm64
and x86_64.

Dropping x86_64 takes it to 30.7 MB unpacked / 10.7 MB packed. The slice
directory is renamed to match its contents and the xcframework manifest
updated so Xcode still resolves it; the podspec vendors the whole
xcframework, so nothing else referenced the old name.

Cost: Intel Mac simulator builds are no longer supported by this module.
Apple Silicon simulator, device, and Mac Catalyst are unchanged.

Symbol stripping is not an alternative here — `strip -S` rejects the
gomobile archives with "string table not at the end of the file", a
known consequence of Go's object layout.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
3.0.137 published for 39 of the 40 packages. npm left
@onekeyfe/react-native-bundle-crypto@3.0.137 staged but never committed:
the tarball 404s, the packument never listed the version, and every
republish is rejected with "409 Cannot publish over previously staged
version". That version number is unrecoverable, so the whole set moves
to 3.0.138 rather than letting one package drift out of lockstep.

Carries the bundle-crypto framework slimming (14.9 MB -> 10.7 MB
packed), which is the most likely reason npm routed that one package
through a staging path the other 39 never touched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e retries

Two gaps that turned the 3.0.137 release into a silent partial one.

The workflow took npm's exit code as proof of publication. npm reports
success when it accepts a tarball, which is not the same as the version
becoming available: @onekeyfe/react-native-bundle-crypto@3.0.137 was
staged and never committed, so it was undownloadable AND unrepublishable
("409 Cannot publish over previously staged version"), every step of the
run was green, and the miss was only found later by hand. The new
verify-published step reads each version back from the registry and
fails the run that produced it.

It reads through `?write=true` deliberately. The read-through CDN served
hours-stale package documents during that incident and made three
healthy packages look unpublished, so a naive check would fail good
releases and train people to ignore it.

The second gap: retrying one failed package meant re-running the whole
release, where the 39 already-published ones reject with E403 and the
run goes red before proving anything about the one that mattered. The
`only_workspace` input publishes and verifies just that package.

Verified against the live registry: all 40 packages at 3.0.138 pass,
single-workspace mode passes, an unknown workspace and a missing
dist-tag both exit 1. 13 unit tests cover the pure logic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@huhuanming huhuanming reopened this Sep 16, 2026
@huhuanming
huhuanming merged commit 406f726 into main Sep 16, 2026
4 checks passed
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.

3 participants