fix: harden ldk node teardown on stop

[jvsena42]

Relates to #982 , #986

Upstream issue: synonymdev/ldk-node#94

This PR:

1. Releases the LDK node handle deterministically on stop instead of leaving it to the garbage collector
2. Prevents a cancelled caller from abandoning node teardown halfway
3. Stops tearing the node down and rebuilding it when the app is only briefly backgrounded

Description

Play Vitals reports a SIGABRT on mainnet as the top production crash cluster. Symbolicating it locally against the unstripped libldk_node.so for the shipped ldk-node-android version resolved every frame and gave a clear cause:

#19 ldk_node::runtime::spawn_background_task  (chain-sync task, tokio worker thread)
#14 drop_slow<ElectrumRuntimeClient>          ← last Arc released here
#13 drop_in_place<ldk_node::runtime::Runtime / RuntimeMode>
#12 drop_in_place<tokio::runtime::Runtime → BlockingPool>
#11 tokio blocking/shutdown.rs:51 → panic: "Cannot drop a runtime in a context
    where blocking is not allowed"

The defect itself is upstream: ldk-node's chain-sync task drops the last reference to the Electrum client, which owns the tokio runtime, while running on a worker thread of that same runtime. Release Rust aborts on panic, so the process dies with no catchable error. That is filed as synonymdev/ldk-node#94 and this PR does not fix it.

What this PR fixes is the Android side that made the window wide and the timing unpredictable. Stopping the node cleared the reference but never destroyed the handle, so the native node stayed alive until the garbage collector freed it on the finalizer thread at an arbitrary later moment, potentially while a new node was already running. The handle is now released as part of the stop itself.

The release is bounded rather than unconditionally synchronous. free_node returns in tens of milliseconds for a healthy node, but blocks for tens of seconds when a failed start left the node's background tasks wedged (measured: ~38 ms healthy vs ~40 s wedged). So the release runs on the IO dispatcher and the stop waits on it only up to a short timeout: on the healthy path teardown still finishes before the next startup begins, and on the wedged path the stop returns and lets the release drain in the background instead of blocking the whole lifecycle — including any recovery restart — behind it. This bound is not just an optimization: an earlier revision that always waited inline turned a 0.6s recovery stop into a 40s one and timed out the Electrum "wrong server" E2E test; see the note below.

Teardown was also abandonable. The stop runs through a cancellable context and was triggered from a ViewModel scope, so an activity finishing mid-stop left a live native node orphaned with the lifecycle state stranded. The teardown is now non-cancellable once committed.

Reading ldk-node showed the stop path could not fail the way the code assumed: it reports an error only when the node is already stopped, swallows and logs every internal failure including the background-task shutdown timeouts, and then returns success. A failed stop therefore no longer rethrows and skips the release.

Finally, backgrounding the app stopped the node immediately, so a short task switch cost a full stop plus a full rebuild. The stop from backgrounding is now deferred by a few seconds and cancelled if the app returns, so brief switches leave the node untouched. Only the backgrounding path is deferred; the notification stop action, service teardown, notification worker, and restore migration all still stop immediately. The deferred stop is owned by the repository's process-lifetime scope rather than a ViewModel scope, so it cannot be silently dropped when the activity goes away.

Preview

2-seconds-pause.webm

long-pause-with-channel.webm

QA Notes

Manual Tests

• 1a. Home → background the app and return within ~2s: no node teardown or rebuild, wallet stays responsive.
  • 1b. Background and wait past the window, then return: node stops, then rebuilds and reaches started.
• 2. regression: Background Payments → enable Get paid when Bitkit is closed and Keep Bitkit active in background → background the app: node keeps running, no teardown.
• 3. regression: Node notification → tap Stop: node stops immediately and the app task is removed.
• 4. regression: Relaunch after a notification stop: node rebuilds and reaches started.
• 5. regression: Settings → Lightning → restart node, and Electrum/RGS server change: node stops and restarts normally.

Automated Checks

• Unit tests added: cover deterministic handle release, release when the node is already stopped, and the no-node case in LightningServiceTest.kt.
• Regression coverage added: teardown completing when the caller is cancelled, handle release when the node stop throws, and no double release on consecutive stops in LightningServiceTest.kt. All three fail against the previous implementation.
• Regression coverage added: teardown ordering before a subsequent rebuild, a background and foreground cycle inside the window never stopping the node, and lifecycle state not stranding when the caller is cancelled in LightningRepoTest.kt.
• Unit tests added: deferred stop timing and cancellation on start in LightningRepoTest.kt.
• Verified the new regression tests fail without the fix by temporarily reverting it: 5 of them failed, then passed once restored.
• Device validation on a Pixel 9 emulator with the dev build, backgrounding for 1, 2, 3, 4, 6 and 8 seconds. Under the window the node stays running and start is skipped; past it the log order is always Stopping node… → Node stopped → Building node… → Node started, confirming teardown completes before startup on the healthy path:

  14:07:05.798  Stopping node…
  14:07:06.283  Node stopped
  14:07:08.778  Building node…
  14:07:18.208  Node started
  

• Device validation of the foreground service path: with background payments and keep-active enabled, 12 seconds backgrounded produced no stop at all and the node kept processing LDK events.
• Device validation of the notification stop action, which is immediate rather than deferred and does not double-stop from service teardown:

  11:22:14.087  Received start command action 'STOP_SERVICE_AND_APP'
  11:22:14.096  Received stop service action
  11:22:14.101  onDestroy
  11:22:14.101  Skipping node stop: activity is active
  11:22:14.155  Stopping node…
  11:22:14.708  Node stopped
  

• No Fatal signal, SIGABRT or tombstone appeared in logcat across any of the runs above, including the stop paths that now drive the native handle release.
• Electrum recovery regression, found in CI and reproduced locally against a server that accepts the TCP connection but never completes the TLS handshake (ssl:// pointed at a plain-TCP listener), which is the @settings_10 "wrong Electrum server" condition. Before the release bound, the post-failure recovery stop took ~40s (past the 30s E2E toast wait) because free_node blocked on wedged background tasks; with the bound it returns in ~1s and the error surfaces. Timeline after the fix:

  Stopping node…                          (healthy stop, ~0.6s)
  Node stopped
  Starting node…                          (fails after ~15s, wrong protocol)
  attempting recovery… → Stopping node…
  Node handle release still pending after 1s
  Node stopped                            (recovery no longer blocked)
  Node started                            (previous config restored)
  

• CI: standard compile, unit test, and detekt checks run by the PR bot.

[greptile-apps]

Greptile Summary

This PR makes Lightning node shutdown deterministic and avoids teardown during brief background transitions. The main changes are:

• Releases the native node handle during shutdown.
• Keeps committed teardown running if its caller is cancelled.
• Defers lifecycle-driven stops and cancels them on foreground startup.
• Adds tests for teardown, cancellation, ordering, and debounce behavior.

Confidence Score: 5/5

This looks safe to merge.

• Pending-stop installation and cancellation now use the same lock.
• Foreground startup cannot miss a job while it is being stored.
• The lifecycle mutex keeps committed teardown ordered with startup.
• No blocking issues were found in the changed code.

Important Files Changed

Filename	Overview
app/src/main/java/to/bitkit/repositories/LightningRepo.kt	Adds synchronized deferred-stop management and makes lifecycle teardown non-cancellable.
app/src/main/java/to/bitkit/services/LightningService.kt	Stops the node and releases its native handle through a bounded asynchronous teardown path.
app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt	Routes lifecycle-driven shutdown through the repository debounce.
app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt	Adds tests for debounce timing, foreground cancellation, lifecycle state, and stop-start ordering.
app/src/test/java/to/bitkit/services/LightningServiceTest.kt	Adds tests for native handle release across normal, cancelled, repeated, and failed stop paths.

_{Reviews (2): Last reviewed commit: "fix: call release independently rather t..." | Re-trigger Greptile}

[jvsena42]

iOS port assessment: not needed

Checked this against the iOS teardown paths (LightningService.stop, WalletViewModel.stopLightningNode, AppScene.handleScenePhaseChange). This PR's three changes each target a mechanism that is Android-specific and structurally absent on iOS, so there is nothing to port.

This PR's change	Android mechanism	iOS
Deterministic handle release (node.destroy(), bounded)	JVM cleared the reference but left the native node to the GC finalizer, freed at an arbitrary later moment, potentially racing a new node	ARC: dropping the last reference at self.node = nil frees the handle deterministically. No finalizer thread, no arbitrary-timing race, so no explicit release to add
Non-cancellable teardown (withContext(NonCancellable))	stop() ran in a cancellable context from a viewModelScope, so an activity finishing mid-stop orphaned a live native node	Swift task cancellation is cooperative and does not interrupt the synchronous FFI node.stop(). stop() also unlocks via defer, and no Activity-like scope tears it down
Deferred stop on backgrounding (stopDebounced)	Backgrounding stopped the node immediately, so a short task switch cost a full stop + rebuild — this is what made the race window wide	iOS never stops the node on backgrounding. handleScenePhaseChange only handles PIN lock and foreground reconnect; the only stop callers are resetNetworkGraph, wipe, and internal restart/recovery. No churn, nothing to debounce

The one genuinely shared item is the underlying defect, synonymdev/ldk-node#94 — which this PR explicitly does not fix, it only narrows the timing window. That's a Rust-side fix and applies to both platforms whenever it lands upstream; it isn't something to port at the app layer.

One minor, non-blocking note for the iOS side: listenForEvents holds a local strong node while parked at await node.nextEventAsync(), so after self.node = nil the actual free is deferred until that task unwinds rather than happening exactly at the assignment. It's far more bounded than a GC finalizer and there is no rebuild churn to race against, so it does not reproduce this crash — just the one place where iOS deallocation isn't strictly deterministic, if we ever want to tighten it.