Skip to content

Conversation

@ovitrif
Copy link
Collaborator

@ovitrif ovitrif commented Jan 24, 2026

Summary

  • Make LightningService.stop() idempotent (succeed if node is already null instead of throwing)
  • Pass node reference to listenForEvents to use a stable reference throughout the event loop
  • Handle node being stopped gracefully in event listener (exit loop instead of crashing)
  • Handle null balances/channels gracefully in getBalancesAsync/getChannelsAsync
  • Always cancel scope in LightningNodeService.onDestroy() since stop is now idempotent

Context

This addresses pre-existing issues exposed by the crash logs shared in PR #717:

  1. getBalancesAsync crash: checkNotNull(lightningService.balances) threw IllegalStateException='Required value was null.' because node?.listBalances() returned null when the underlying LDK node was stopped
  2. Non-idempotent stop: LightningService.stop() threw NodeNotStarted when node was already null
  3. Event listener instability: listenForEvents accessed this.node on each loop iteration, which could return null mid-loop when stop was called concurrently

Note: The node property is already @Volatile which provides sufficient thread-safety for the reference itself. These are not thread-safety issues with the reference - they're about handling the case when the LDK node object is stopped and its methods return null.

Test plan

  • Build succeeds: ./gradlew compileDevDebugKotlin
  • Unit tests pass: ./gradlew testDevDebugUnitTest
  • Lint clean: ./gradlew detekt
  • Manual test: verify the crashes reported on parent PR Handle node connection issues #717 dont repro

@claude

This comment has been minimized.

@ovitrif ovitrif force-pushed the fix/handle-node-connection-errors-review branch 2 times, most recently from ba90569 to 4c4ef7a Compare January 24, 2026 14:50
@ovitrif ovitrif changed the title fix: make node stop idempotent and fix race conditions fix: make node stop idempotent and handle null gracefully Jan 24, 2026
@ovitrif ovitrif changed the title fix: make node stop idempotent and handle null gracefully fix: handle node null gracefully and stop idempotently Jan 24, 2026
@ovitrif ovitrif changed the title fix: handle node null gracefully and stop idempotently fix: handle null node gracefully and stop idempotently Jan 24, 2026
@ovitrif ovitrif changed the title fix: handle null node gracefully and stop idempotently fix: graceful null handling and idempotent stop Jan 24, 2026
@ovitrif ovitrif force-pushed the fix/handle-node-connection-errors-review branch from 4c4ef7a to 24ef3da Compare January 24, 2026 20:07
- Make LightningService.stop() idempotent (succeed if node is null)
- Pass node reference to listenForEvents to avoid accessing stale this.node
- Handle node being stopped gracefully in event listener
- Handle null balances/channels gracefully instead of crashing
- Always cancel scope in LightningNodeService.onDestroy()
\
@ovitrif ovitrif force-pushed the fix/handle-node-connection-errors-review branch from 24ef3da to 65230bc Compare January 24, 2026 20:46
@claude
Copy link

claude bot commented Jan 24, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

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.

2 participants