Skip to content

fix: exit instead of releasing a detached node - #1838

Merged
jvsena42 merged 5 commits into
masterfrom
fix/detached-node-runtime-exit
Nov 10, 2026
Merged

jvsena42 merged 5 commits into
masterfrom
fix/detached-node-runtime-exit

Conversation

@jvsena42

@jvsena42 jvsena42 commented Nov 10, 2026 •

Copy link
Copy Markdown
Member

Closes #1844

This PR ends the app process, instead of releasing the node, when a node stop in the background leaves ldk-node tasks detached. This is step 1 of the plan in #1536 (#1536 (comment)). It mitigates the crash in the app. It does not fix ldk-node, so #1536 stays open.

Description

  • When a stop times out, ldk-node detaches the tracked tasks it could not cancel, such as the chain-sync loop stuck in a VSS write through block_in_place. Releasing the node (destroy → Runtime::drop → shutdown_background) then shuts down the runtime underneath them. The first one that resumes polls a timer on the dead runtime and aborts with A Tokio 1.x context was found, but it is being shutdown. Resuming means the VSS write returning or the process thawing from the freezer.
  • Adds NodeRuntimeMonitor, one per node build. It flags the node when ldk-node logs an ERROR from ldk_node::runtime starting with Detaching (background tasks, cancellable background tasks or the event processor). ldk-node exposes this only through its log. LdkLogWriter takes an onRecord hook, called before the level filter.
  • In LightningService.stop(), after node.stop(), a flagged node in a backgrounded app logs the reason, waits 500 ms for the log writer, checks again that the app is still in the background, then calls Process.killProcess. If the user came back during the wait, the node is released as in the foreground.
  • "Backgrounded" means no started activity, from a counter added to App.CurrentActivity. A paused activity behind a system dialog still counts as visible. ProcessLifecycleOwner can't be used: the manifest removes androidx.startup's InitializationProvider, so it never initializes and always reads as background. The runtime is never shut down, so the detached tasks can't abort the app, and the next launch is a cold start. The kill skips exit(), so no native teardown runs against the live runtime.
  • In the foreground the node is released as before. Ending the process there would close the app in front of the user.

Out of Scope

Design

N/A — no UI changes.

Preview

N/A

QA Notes

Setup

  • Emulator: an AVD with the cached-apps freezer disabled (adb shell settings put global cached_apps_freezer disabled, then reboot), so the abort shows up while the app is backgrounded instead of on thaw.
  • Local backend from bitkit-docker (vss-server on 5050, lnurl-auth-server on 5005, electrum on 60001).
  • A TCP proxy on host port 15050 that forwards to 127.0.0.1:5050, and while a flag file exists stops forwarding bytes without closing the connection. Requests through it hang, like the iptables DROP recipe in crash: native abort when the node restarts after returning from the background #1536, without root.
  • Build: E2E=true E2E_VSS_SERVER_URL=http://localhost:15050/vss E2E_LNURL_AUTH_SERVER_URL=http://localhost:5005/auth ./gradlew assembleDevDebug, then adb reverse tcp:15050 tcp:15050, adb reverse tcp:5005 tcp:5005 and adb reverse tcp:60001 tcp:60001.
  • Fresh wallet (New Wallet), wait for the first SyncCompleted.

Journeys

N/A — not drivable; see Manual Tests.

Manual Tests

  • 1. Node running on Home → stall the VSS proxy → tap the balance (pending SETTINGS backup) → press Home → logcat shows Detaching background tasks after cancellation timed out., then Exiting process: node stop left ldk_node tasks detached while in background; the process is gone and no Fatal signal 6 follows. Unstall, reopen: the app cold-starts to Home and the node runs. — needs a VSS stall and logcat, not in Capabilities
  • 2. Same as 1 with VSS not stalled → the stop finishes, Node stopped is logged, the process stays alive. — needs logcat, not in Capabilities
  • 3. App in the foreground on Settings → Advanced → Dev Settings → LDK → stall the VSS proxy, wait 20 s → tap Restart and stay in the app → logcat shows Detaching background tasks after cancellation timed out. and Node stopped, no Exiting process, and the app stays open with the same PID. — needs a VSS stall and logcat, not in Capabilities

Automated Checks

  • added NodeRuntimeMonitorTest.kt — flags each ldk-node detaching message, ignores timeouts without a detach and other modules or levels, and LdkLogWriter forwards records to it
  • added CurrentActivityTest.kt — counts started activities, keeps a paused activity visible, ignores an unmatched stop
  • updated LightningServiceTest.kt — stop kills the process instead of releasing a flagged node in the background, releases it in the foreground, releases an unflagged node in the background, and releases a flagged node when the app returns during the delay

Evidence

Run on d0b1e99d1 (fix) and master 176430965, devDebug e2e builds with the setup above, emulator Gate_Throwaway (API 37.1, google_apis_playstore, freezer disabled), ldk-node 0.7.0-rc.79, 10 Nov 2026. VSS was stalled for 240 s from just before Home, which outlasts the VSS client's 180 s retry budget, so the stuck write fails by itself after the stop.

Build Runs Stop detached tasks Outcome
master 4 (1 × 90 s stall, 3 × 240 s) 4/4 process alive, released runtime; no abort seen
this PR 3 (240 s stall) 3/3 Exiting process… then Process to.bitkit.dev … has died 0.6 s later; no Fatal signal 6
  • 1: passed 3/3 on d0b1e99d1. The cycle after each kill cold-started the app and the node reached SyncCompleted.
  • 2: passed on d0b1e99d1. Node stopped 21 ms after Stopping node…, process still alive.
  • Every stalled stop logged the same sequence as the issue: Stopping background tasks timed out. → Detaching background tasks after cancellation timed out. → Stopping event handling timed out.
  • An earlier attempt with airplane mode instead of a stall (4 master runs) timed out the stop but never detached tasks. VSS writes fail fast there and cancellation wins.
  • Rerun on 63eb5fabe after the background check changed (Gate_Throwaway, same proxied local VSS setup, 60 s stall, ldk-node 0.7.0-rc.79):
    • 1: passed. A Home press with tasks detached logged Exiting process…, then Process to.bitkit.dev (pid 4774) has died 0.7 s later.
    • 3: passed. Dev Settings → LDK → Restart in the foreground with VSS stalled: Detaching background tasks…, then Node stopped, no exit, same PID (3026) afterwards.
    • just compile, just test and just lint passed on 63eb5fabe. 7460ccfbe changes only the changelog sentence.
  • Earlier runs, below, were on d0b1e99d1, where the background check was ProcessLifecycleOwner and always read as background. Their background results still hold. No foreground case ran on that head.
  • just compile, just test and just lint passed on d0b1e99d1.

Gaps

  • The abort itself did not reproduce on master in 4 runs with detached tasks, so these runs show the fix ending the process at the right point, not a before/after crash count. The issue's emulator runs also hit 0 of 8, and Play Vitals and the reviewer's runs on fix: decouple wallet restore from profile refresh #1686 are the evidence that it happens.
  • Not run with the freezer enabled, or on a physical device.
  • Stops in the foreground are unchanged; see Out of Scope. That includes the likely common shape: the user backgrounds, the stop stalls, and the user returns before it ends. The stop then sees the app in the foreground and releases the node, and nothing kills the process if the app is backgrounded again later.
  • With the cached-apps freezer on, a stalled background stop can reach its decision only when the process thaws. If the user's return is what thaws it and onActivityStarted arrives more than 500 ms after the first check, the app restarts instead of resuming. If it arrives in time, the node is released and the original abort can still happen. The device runs had the freezer off.
  • Backgrounding during a stalled wipe or Electrum/RGS change kills the process mid-operation. Nothing is lost, since nothing local is deleted and the new server URL is saved only on success, so the wallet comes back un-wiped or on the old server.
  • Detection depends on ldk-node's log wording (Detaching … from ldk_node::runtime). A wording change in a future ldk-node silently disables the guard; NodeRuntimeMonitorTest.kt pins the current messages.

Models used

  • Planning/scoping: claude-opus-5-5 (reasoning: Not exposed)
  • Implementation: claude-opus-5-5 (reasoning: Not exposed)
  • Review: claude-opus-5-5 (reasoning: Not exposed)
  • Review rounds: 3

Metadata note: the session and the independent reviewer subagent both report the model ID claude-opus-5-5; neither exposes a reasoning-effort setting.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 7460ccf (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jvsena42
jvsena42 marked this pull request as ready for review November 10, 2026 16:04
@jvsena42
jvsena42 requested review from a team, ben-kaufman and coreyphillips and removed request for a team November 10, 2026 16:04
@greptile-apps

greptile-apps Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge within its stated background-stop scope.

Summary

The PR ends the background app process when stopping a Lightning node leaves tasks detached, instead of releasing that node.

  • Background stops with detached node work now end the app process.

The earlier, unnumbered finding about returning during the delay is addressed. jvsena42 explicitly leaves detached-node crashes during foreground release out of scope.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Node stop finishes] --> B{Detached tasks and app backgrounded?}
  B -->|No| F[Release node]
  B -->|Yes| C[Wait 500 ms for logs]
  C --> D{App still backgrounded?}
  D -->|No| F
  D -->|Yes| E[Kill process without releasing node]
Loading

Reviews (2) · Last reviewed commit: "fix: detect background from started acti..." · Reviewed by Greptile

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ovitrif

ovitrif commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

Placed in the 3.2.0 milestone through #1844, which this PR now closes. It mitigates the background-restart crash in #1536, which is already planned for 3.2.0. #1536 stays open for the ldk-node fix. It has no iOS twin: synonymdev/bitkit-ios#1269 was closed because iOS does not release the node this way.

@talosmachina talosmachina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 finding (1 P1). Kills the process instead of releasing the node when a background stop leaves ldk-node tasks detached, so the runtime is never shut down under them (#1536). Reviewed b04e632, full tier, static.

What I checked, and 4 candidates I ruled out

Read in full: LightningService.kt (stop, setup, releaseHandle, awaitNodeRelease), LoggerLdk.kt, both test files, AndroidManifest.xml, App.kt, LightningNodeService.kt, LightningRepo.stop/resetNetworkGraph, WipeWalletUseCase
Cross-checked: ldk-node v0.7.0-rc.79 src/runtime.rs and src/logger.rs: the three Detaching ... messages are log_error! from ldk_node::runtime, and the custom writer gets every record before any level filter, so the monitor matches what the pinned ldk-node logs.
Build: not built. On the Bitkit track an auto review does not build an Android branch here; reasoned from the code, the pinned ldk-node source and CI. d0b1e99 passed CI and Lint; on b04e632, CI's build gate failed because its build jobs were cancelled while a newer run was in progress, so no result yet for this head (the delta is a moved comment).

Ruled out

  • Monitor swap racing a stop: stop() captures runtimeMonitor before node.stop(), and setup() waits on the LDK queue, so a rebuild cannot swap the monitor mid-stop.
  • A stale flag on the next node: each setup() makes a fresh NodeRuntimeMonitor, and a killed process starts clean.
  • Kill racing the user coming back during the 500 ms flush (Greptile's thread): already raised; narrow next to the finding above, which decides the background check anyway.
  • LdkLogWriter used elsewhere without the hook: the default onRecord = {} keeps other call sites unchanged.
  • Cold reader: the finding stood; it rated it P2, I kept P1 because it closes the app in front of the user on a path the PR says stays unchanged.

Merge confidence: 3/5, one P1.

Reviewer model: claude-opus-5-5
Reasoning effort: Not exposed (the run metadata available to this reviewer does not expose a setting)

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two independent reviews.

needs changing before merge

  • isProcessInBackground() is always true, so foreground stops kill the app (app/src/main/java/to/bitkit/services/LightningService.kt:1620-1621)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jvsena42

Copy link
Copy Markdown
Member Author

@greptileai review

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked after 2 new commits (63eb5fa, 7460ccf). Nothing blocking remains.

addressed

  • isProcessInBackground() is always true, so foreground stops kill the app (app/src/main/java/to/bitkit/services/LightningService.kt:1620-1621). Fixed: LightningService.kt:1619 now reads App.currentActivity?.hasStartedActivity != true, and both checks (lines 565 and 573) use it, with no ProcessLifecycleOwner left in LightningService. The counter at App.kt:103-122 goes up in onActivityStarted and down in onActivityStopped, and App.onCreate registers it at App.kt:62 before any activity starts. A paused but visible activity therefore counts as foreground, and an A-to-B transition starts B before stopping A, so there is no zero gap. I read the callbacks and the single-process manifest, confirmed CurrentActivityTest.kt covers the counter and that CI build-and-test passes on 7460ccf, and noted your device case 3 (foreground restart, same PID). I did not rerun it on a device.

@jvsena42
jvsena42 enabled auto-merge (squash) November 10, 2026 17:14
@jvsena42
jvsena42 merged commit ac1796a into master Nov 10, 2026
23 checks passed
@jvsena42
jvsena42 deleted the fix/detached-node-runtime-exit branch November 10, 2026 17:21
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.

crash: abort after releasing a detached node

4 participants