Fix the Android Zano freeze, deadlock, and file recovery by peachbits · Pull Request #20 · EdgeApp/react-native-zano

peachbits · 2026-08-27T00:05:46Z

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

Yes
No

Dependencies

none — supersedes EdgeApp/edge-currency-accountbased#1094: with the deadlock
fixed at its source here, accountbased needs no changes at all, and
checkpointing stays enabled with identical code on both platforms.

Description

Fixes the Android freeze QA reported — about seven minutes after login, taps
and buttons die while scrolling and the drawer keep working, and the Zano
sync banner stops updating; iOS is unaffected — in three commits: one
contains the blast radius of any blocked Zano call, one removes the deadlock
that produced the blocked call, and one fixes a wallet-file recovery gap the
deadlock's verification uncovered.

Commit 1 — keep Zano calls off the RN bridge thread.

On the legacy Android architecture every @ReactMethod runs on the single
shared mqt_native_modules thread, and callZano ran its JNI synchronously
there. That thread also executes UIManager's view commands, so any Zano call
that blocks in C++ freezes every view update in the app: buttons dead
(UIManager), sync banner frozen (every native call queued behind the blocker,
including the tryPullResult polls that would have observed the close
finishing), scroll and drawer alive (pure UI thread). QA's screen recording
matches this signature exactly — the sync banner reads the same block count
across four minutes.

Repro evidence (API 35 emulator, checkpoints tortured to a 30s interval,
4 wallets in deep catch-up): a JDWP thread dump captured mid-freeze shows

mqt_native_modules:
  [1] app.edge.rnzano.RnZanoModule.callZanoJNI (native method)
  [2] app.edge.rnzano.RnZanoModule.callZano (RnZanoModule.java:46)

callZano now hops to a dedicated single-thread executor and settles the
promise from there. Argument extraction from the ReadableArray stays on the
caller thread, and a single thread preserves the strict global call ordering
the shared bridge thread provided. Re-running the identical torture scenario
with this change, the blocked call sits quarantined on the executor thread
while mqt_native_modules, mqt_js, and main all stay idle and rendering
continues.

Per review: the executor is static — the Zano SDK is one instance per
process while React module instances are not, so a JS reload would otherwise
run the old instance's still-draining call concurrently with the new
instance's calls (with commit 2, that overlap could double-open a wallet
file mid-store). The thread is named "zano" so it identifies itself in
future dumps, and the catch widens to Throwable, since off the bridge
thread an escaping Error would kill the process with the promise
unsettled. The change needs no architecture detection: it never blocks the
thread that called it, which is correct under both the legacy bridge and
the new-arch interop dispatch.

Commit 2 — patch the SDK's close-during-scan deadlock.

The call that blocked is close_wallet, issued by the engine's mid-sync
checkpoint save. The pinned SDK implementation holds m_wallets_lock
exclusively across two waits on the wallet being closed: the store() call,
which needs the per-wallet lock the refresh worker holds for the whole of a
scan chunk, and the map erase, whose destructor joins the worker thread. The
refresh path re-enters the manager through wallet callbacks that take
m_wallets_lock shared (the SDK documents the ordering hazard in its
on_sync_progress comment), and every other API call takes it shared up
front — so one close issued mid-catch-up wedges the worker, the close, and
then every Zano call in the process, permanently. In two of two torture runs
the wedge never cleared and no checkpoint ever landed again.

update-sources now rewrites close_wallet in the downloaded sources:
detach the map node while holding the lock, then do the store and the
implicit worker join with no manager lock held. Node extraction keeps the
element at its address, so the worker's references stay valid, and wallet
ids are never reused. The stop flags, the store-then-log-prefix order, and
the return codes are kept verbatim; the log-prefix write gains a bounds
check, since the manager lock no longer serializes it against reset()'s
vector clear. The transform requires the found function to match the pinned
original exactly (modulo trailing whitespace), so a pin bump that changes
close_wallet in any way fails the build for a human to re-evaluate the
patch instead of silently keeping or dropping it
(scripts/utils/closeWalletPatch.ts, unit-tested).

Two deliberate semantics changes, documented in the transform: during the
store/join window the wallet is absent from the map while its wallet2
still writes the file — safe for this bridge, which strictly sequences
close-before-reopen per wallet on one executor thread, and called out for
any upstream submission. And when store() throws, the original left the
entry in the map as a zombie (stop flags set, never able to sync again),
while the rewrite reports the same error with the wallet gone.

Only Android builds from these sources; iOS links the prebuilt
libzano-plain-wallet xcframework and is byte-for-byte unchanged. iOS has
not exhibited the wedge across hundreds of observed checkpoint cycles,
though the lock inversion exists in its prebuilt code too — the durable fix
for both platforms is handing this patch upstream with the next pin-bump
request.

Commit 3 — rebuild unreadable wallet files.

Found while verifying the deadlock fix: a crash during a wallet file's very
first write leaves a file the SDK cannot parse — typically zero bytes — and
opening it fails with INVALID_FILE before any password is consulted
(wallet2's header read maps to API_RETURN_CODE_INVALID_FILE).
startWallet's recovery ladder is keyed on WRONG_PASSWORD, so the error
propagated as-is and the engine retried the same doomed open every second,
forever — a full native init and load per attempt, per wallet, with no path
back to health. Observed live after an emulator was hard-killed mid-restore:
two of four wallets left zero-byte files and spun at 28 failed opens per 20
seconds while the other two synced.

INVALID_FILE now routes to the same policy as a file no known password
opens, skipping the pointless password ladder: with no seed passphrase set,
the file is deleted and rebuilt from the mnemonic, recreating the identical
wallet at the cost of a re-scan; with a passphrase set, it refuses with a
clear error, since an unreadable file cannot corroborate the passphrase and
a wrong one would rebuild a different wallet. This half is platform-neutral
JS. On-device: both stuck wallets logged the new path, rebuilt, and synced;
the error spam went to zero.

Verification. Unit suite green (53 tests). A full update-sources run
from a fresh checkout applies the patch and rebuilds all four Android ABIs
and the iOS xcframework cleanly, with wallets_manager.cpp recompiled per
ABI. The emulator torture repro that wedged 2/2 before the patch — 30-second
checkpoint interval, four wallets in deep catch-up — ran 25 minutes against
the rebuilt library: 41 completed checkpoint cycles, zero stalls, the
app process alive and interactive throughout, with each cycle also
re-opening the file the previous patched close wrote. A later session added
stock-interval (5-minute) checkpoint cycles landing on schedule.

j0ntz

Neither of these blocks the PR. RnMoneroModule in react-native-monero-lwsf uses the identical executor pattern, so both apply there too.

j0ntz · 2026-08-27T04:05:31Z

+  // scan chunk, or the SDK's close-during-scan lock inversion) froze every
+  // view update in the app. A single thread preserves the strict global
+  // call ordering the shared bridge thread provided.
+  private final ExecutorService executor = Executors.newSingleThreadExecutor();


Warning: scoping the executor to the module instance narrows the ordering guarantee the comment above claims.

mqt_native_modules is one thread per process, so its ordering held across React context recreations. An instance field does not. A JS reload builds a new RnZanoModule with a second executor while the first one's thread is still alive and possibly still draining queued calls, so two threads then call into the same process-global Zano SDK. That is the concurrent close-during-scan shape this PR exists to contain.

sequenceDiagram participant JS participant Exec1 as executor, instance 1 participant Exec2 as executor, instance 2 participant SDK as Zano SDK, process-global JS->>Exec1: closeWallet(w) Note over Exec1,SDK: blocked in C++ JS->>JS: reload rebuilds the ReactContext JS->>Exec2: open(w) Exec2->>SDK: open Exec1->>SDK: closeWallet resumes Note over SDK: two threads, ordering gone

Loading

private static final ExecutorService executor restores the process-global property and drops the per-reload thread leak in the same line. An invalidate() override calling shutdown() fixes only the leak: the in-flight call still races the new instance.

Same declaration, separate nit: Executors.newSingleThreadExecutor(r -> new Thread(r, "zano")) names the thread. The evidence in this PR's description is a thread dump, and pool-1-thread-1 will not identify itself in the next one.

Both taken, in ca1dc61: the executor is now static final, and the thread is named "zano" so it identifies itself in the next dump.

One sharpening on the mechanism from re-checking RN's source: mqt_native_modules is per-CatalystInstance rather than per-process, so the pre-PR ordering did not actually survive reloads either. That makes the static executor a strengthening rather than a restoration -- and your scenario is the one that makes it load-bearing rather than hygiene: with the close_wallet patch in this PR, a mid-close wallet is detached from the manager's map before its store lands, so a reload-spawned second thread could double-open the very file the old thread is still writing. The instance-scoped executor was the only thing standing between those two facts.

The same pair of fixes for RnMoneroModule in react-native-monero-lwsf is queued as a follow-up, per your note.

j0ntz · 2026-08-27T04:05:31Z

+    executor.execute(() -> {
+      try {
+        promise.resolve(callZanoJNI(method, strings));
+      } catch (Exception e) {


Suggestion: catch (Exception e) no longer covers everything the promise depends on.

The clause is unchanged, but the thread under it is not. On mqt_native_modules an escaping Error reached RN's NativeModuleCallExceptionHandler (redbox in dev, an RN-owned crash in prod). Out of an executor.execute Runnable it goes to the default uncaught handler instead, which kills the process. promise was never settled in that path before either; catch (Throwable e) costs one word and gets the JS caller a ZanoError instead of silence.

Done in ca1dc61 -- catch (Throwable), with a comment carrying the rationale: off the bridge thread an escaping Error reaches the default uncaught handler and kills the process with the promise unsettled, where RN's own dispatcher used to own it.

@ReactMethod

On the legacy Android architecture every @ReactMethod runs on the single shared mqt_native_modules thread, which also executes UIManager's view commands, so a Zano call that blocked in C++ froze every view update in the app: taps dead, screens frozen, native scrolling alive. A JDWP dump of the frozen app shows callZano parked on that thread inside callZanoJNI. callZano now hops to a dedicated single-thread executor and settles the promise from there. The executor is static: the Zano SDK is one instance per process while React module instances are not, so a JS reload would otherwise run the old instance's still-draining call concurrently with the new instance's calls. A single process-wide thread preserves the strict global call ordering across reloads too, and is named "zano" so it identifies itself in future dumps. The catch widens to Throwable: off the bridge thread an escaping Error would hit the default uncaught handler and kill the process with the promise unsettled, where RN's dispatcher previously caught it. Argument extraction from the ReadableArray stays on the caller thread.

j0ntz

Read the executor change and the INVALID_FILE recovery too, and both look right to me. The notes below are all on the close_wallet patch.

j0ntz · 2026-08-27T22:43:32Z

+
+    it->second.major_stop = true;
+    it->second.stop_for_refresh = true;
+    it->second.w.unlocked_get()->stop();


Warning: stop() moved out of the try/catch, so its failures change shape at the JS boundary.

In the pinned original this call sat inside the try alongside store() and the erase, so a throw came back as API_RETURN_CODE_FAIL:<what> or API_RETURN_CODE_INTERNAL_ERROR. Here it runs in the lock scope that closes on line 117, before the try opens on line 119. A throw now unwinds out of close_wallet (the RAII lock still releases, so no deadlock) into the catch-all at src/jni/jni.cpp:47-59, which ThrowNews a Java exception. That reaches JS as a rejected promise rather than a resolved { response: 'FAIL:...' }.

Every caller is written against the return-code shape: CppBridge.ts:417 decides from closeResponse whether deleting the file is safe, and lines 596 and 624 branch on response !== 'OK'. It also contradicts the docstring above, which says the deliberate semantics changes are "all confined to the window after the node leaves the map" (line 71). This one happens before the extract.

Opening the try before the lock scope, with EXCLUSIVE_CRITICAL_REGION_LOCAL inside it, restores the original contract without touching the deadlock fix. Nothing in test/closeWalletPatch.test.ts pins it either way.

Fixed in ea9638d (now 67a8fce after the rebase): the try opens before the lock scope, so stop(), the extract and the find are back inside it and failures keep reporting as return codes. The node stays declared outside the try, so the worker join in its destructor still runs at function exit rather than during unwinding -- a join() throwing mid-unwind would be a std::terminate the original could not produce.

You are right that the docstring was wrong as written; it now says the changes are confined to the window after the extract, which is true again. Added a test that walks the patched body and asserts every throwing step sits after the try and the node declaration sits before it.

j0ntz · 2026-08-27T22:43:33Z

+ * leaves the map:
+ *
+ * - "Absent from `m_wallets`" no longer implies "closed and stored".
+ *   During the store/join window the wallet is invisible to


Question: does this list need a third bullet for the refresh worker's own callbacks?

The deadlock analysis above names on_transfer2 and on_transfer_canceled as the path by which the refresh worker re-enters the manager and takes m_wallets_lock shared. Those callbacks fire during exactly the store/join window this bullet describes, when the node is out of m_wallets, so a lookup by wallet_id there now misses where it previously found the entry. The bullet covers open_wallet's ALREADY_EXISTS check and status calls, but not the one caller guaranteed to be running.

I do not have the SDK sources checked out to see what that path does with a miss. A dropped notification during a close is harmless; an unchecked ->second on end() is not. If it is safe, saying so in this list would settle it for the next reader and for the upstream submission this docstring is already drafting.

Checked it against the pinned sources: safe, and now written into the list as a third bullet.

on_transfer2 goes through GET_WALLET_OPTIONS_BY_ID_VOID_RET (wallets_manager.cpp:37-42), which does a checked find and returns on a miss; on_transfer_canceled has its own end() test that logs and returns. So a callback landing in the store/join window drops a view notification for a wallet that is closing anyway. Every m_wallets.find in the file is checked the same way, and the three bare m_wallets[...] sites all insert a freshly counted id in the open, restore and generate paths -- ids come from a monotonic counter, so a late callback cannot hit a recycled one either.

j0ntz · 2026-08-27T22:43:33Z

+function normalize(code: string): string {
+  return code
+    .split('\n')
+    .map(line => line.replace(/[ \t]+$/, ''))


Suggestion: normalize does not strip \r.

The regex runs per line after split('\n'), so a CRLF checkout leaves a trailing \r on every line, and the byte-for-byte comparison then fails against the LF-only original on unmodified upstream source. The build stops with a message pointing at a drift that did not happen. /[ \t\r]+$/ covers it.

Low odds on macOS and Linux defaults, and zano_native_lib may well ship a .gitattributes, so this is a one-character hedge rather than a live bug.

Fixed -- the class is /[ \t\r]+$/ now, with a test that feeds the transform a CRLF copy of the pinned source and expects it to patch rather than report drift.

The SDK's close_wallet holds m_wallets_lock exclusively across two waits on the wallet being closed: the store() call, which needs the per-wallet lock the refresh worker holds for the whole of a scan chunk, and the map erase, whose destructor joins the worker thread. The refresh path re-enters the manager through wallet callbacks that take m_wallets_lock shared, so a close issued while the wallet is catching up wedges the worker, the close, and then every Zano call in the process, permanently. The app's periodic mid-sync checkpoint saves trip this within minutes of importing a wallet on Android. update-sources now rewrites close_wallet in the downloaded sources to detach the map node under the lock and do the store and the join with no manager lock held. Everything that can throw stays inside the try, so failures keep reporting as return codes rather than escaping into the JNI catch-all, which callers do not expect; the node is declared outside it so the worker join in its destructor runs at function exit rather than during unwinding. The log-prefix write gains a bounds check: the manager lock no longer serializes it against reset()'s vector clear, so the blind vector[wallet_id] store could go out of bounds. The transform requires the found function to match the pinned original exactly, modulo trailing whitespace and line endings, so a pin bump that changes close_wallet in any way fails the build for a human to re-evaluate the patch instead of silently keeping or dropping it. Only Android builds from these sources; iOS links the prebuilt xcframework and keeps the original blocking semantics.

A crash during a wallet file's very first write leaves a file the SDK cannot parse -- typically zero bytes -- and opening it fails with INVALID_FILE before any password is consulted. startWallet's recovery ladder is keyed on WRONG_PASSWORD, so the error propagated as-is and the engine retried the same doomed open every second, forever: a full native init and load per attempt, per wallet, with no path back to health. Observed live after an emulator was hard-killed mid-restore: two of four wallets left zero-byte files and spun in the retry loop while the other two synced. Route INVALID_FILE to the same policy as a file no known password opens, skipping the pointless password ladder: with no seed passphrase set, delete the file and rebuild it from the mnemonic, which recreates the identical wallet at the cost of a re-scan; with a passphrase set, refuse with a clear error, since an unreadable file cannot corroborate the passphrase and a wrong one would rebuild a different wallet.

peachbits mentioned this pull request Aug 27, 2026

Disable Zano catch-up checkpoints on Android EdgeApp/edge-currency-accountbased#1094

Closed

2 tasks

peachbits marked this pull request as ready for review August 27, 2026 00:11

j0ntz approved these changes Aug 27, 2026

View reviewed changes

j0ntz reviewed Aug 27, 2026

View reviewed changes

peachbits changed the title ~~Keep Zano calls off the RN bridge thread~~ Fix the Android Zano freeze and close deadlock Aug 27, 2026

peachbits force-pushed the zano-android-executor branch from bc65a54 to 40c3ad5 Compare August 27, 2026 21:07

peachbits changed the title ~~Fix the Android Zano freeze and close deadlock~~ Fix the Android Zano freeze, deadlock, and file recovery Aug 27, 2026

j0ntz reviewed Aug 27, 2026

View reviewed changes

peachbits added 2 commits August 27, 2026 16:02

peachbits force-pushed the zano-android-executor branch from c64eece to 67a8fce Compare August 27, 2026 23:02

Navigation Menu

Uh oh!

Fix the Android Zano freeze, deadlock, and file recovery - #20

Fix the Android Zano freeze, deadlock, and file recovery#20
peachbits wants to merge 3 commits into
mainfrom
zano-android-executor

peachbits commented Aug 27, 2026 •

edited

Loading

Uh oh!

j0ntz left a comment

Uh oh!

j0ntz Aug 27, 2026

Uh oh!

peachbits Aug 27, 2026

Uh oh!

j0ntz Aug 27, 2026

Uh oh!

peachbits Aug 27, 2026

Uh oh!

j0ntz left a comment

Uh oh!

j0ntz Aug 27, 2026

Uh oh!

peachbits Aug 27, 2026

Uh oh!

j0ntz Aug 27, 2026

Uh oh!

peachbits Aug 27, 2026

Uh oh!

j0ntz Aug 27, 2026

Uh oh!

peachbits Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

peachbits commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CHANGELOG

Dependencies

Description

Uh oh!

j0ntz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

j0ntz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

peachbits commented Aug 27, 2026 •

edited

Loading