fix: sigabrt runtime teardown - #111

Merged
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown
Sep 2, 2026
Merged

fix: sigabrt runtime teardown#111
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown

Conversation

@ovitrif

@ovitrifovitrif commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes#110

Summary

  • Make Node the sole owner of the Tokio runtime and give tasks and exported child handles non-owning runtime control.
  • Close task admission, gate Electrum callbacks, and drain or cancel tracked work before runtime disposal.
  • Isolate VSS I/O on a store runtime whose fallback disposal is safe from async contexts.
  • Propagate event-processor persistence failures after cleanup and remove shutdown panic paths.
  • Bump all package surfaces to 0.7.0-rc.66 and regenerate Android, JVM, Python, and Swift artifacts without changing the public or UniFFI APIs.

Validation

  • Local verification: cargo fmt --all -- --check, RUSTFLAGS='-D warnings' cargo build, cargo check --release --features uniffi, and cargo test --features uniffi --lib (107 passed); focused runtime, Electrum, VSS, and release-mode regressions passed.
  • ./bindgen.sh regenerated JVM, Python, Swift, and Android artifacts for 0.7.0-rc.66; package builds, Maven Local publication, Swift checksum, native debug-symbol matching, and 16 KiB alignment checks passed.
  • Android Maven Local validation: the signed bitkit-dev-release-188-universal.apk from devRelease completed 100 consecutive in-app LDK node restarts on an arm64-v8a Android 16/API 36 emulator without SIGABRT or Tokio runtime-shutdown failures.

Release

@ovitrifovitrif self-assigned this Sep 1, 2026
@chatgpt-codex-connector

This comment has been minimized.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif

ovitrif commented Sep 1, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Validate ldk-node PR #111 in Bitkit Android

Objective

Validate the exact native library from synonymdev/ldk-node PR #111 inside a Bitkit Android build before publishing another ldk-node package.

The validation must exercise Bitkit's real node stop, destroy, and rebuild paths. It does not need the ldk-node channel_full_cycle integration test or any splicing flow.

Recorded Android validation

  • ldk-node head: dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Maven Local coordinate: com.synonym:ldk-node-android:0.7.0-rc.65-pr111.dc37559.
  • App build: signed bitkit-dev-release-188-universal.apk from the devRelease build type.
  • Device: arm64-v8a Android 16/API 36 emulator.
  • Lifecycle result: 100 consecutive in-app LDK Debug node restarts completed without SIGABRT or Tokio runtime-shutdown failures.
  • Native provenance: the APK's arm64-v8a libldk_node.so Build ID was 20b0b1bc8e28be7e153b5265e3c3f094c37ae6fa, matching the Maven Local AAR.

Inputs and isolation

  • Use the latest reviewed PR fix: sigabrt runtime teardown #111 head and record its full commit SHA before building. The current head is dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Derive a unique Maven Local version from that head, for example 0.7.0-rc.65-pr111.dc37559, so the artifact cannot be confused with a released rc.65 package.
  • Use a temporary Bitkit Android validation worktree based on the intended app baseline. Keep the existing checkout and unrelated work unchanged.
  • Save commands, dependency evidence, Build IDs, device details, and logs under .ai/logs/<YYYY-MM-DD>/ldk-pr111-android/.

Build and publish the local ldk-node artifact

  1. Confirm the ldk-node worktree is clean and its checked-out SHA is the recorded PR head.
  2. Run scripts/uniffi_bindgen_generate_kotlin_android.sh with ORG_GRADLE_PROJECT_version set to the unique local version.
    • The existing script builds release-smaller native libraries for armeabi-v7a, arm64-v8a, and x86_64.
    • It generates the Kotlin bindings and native debug-symbol archive.
    • It validates stripped libraries and 16 KB alignment.
    • It publishes com.synonym:ldk-node-android:<localVersion> to Maven Local.
  3. Verify Maven Local contains the AAR, POM, sources, Javadoc, and native-debug-symbols artifact for the unique coordinate.
  4. Record the Build ID and SHA-256 of each ABI's libldk_node.so from the AAR and debug-symbol archive.

Consume the artifact from Bitkit Android

  1. Change only gradle/libs.versions.toml in the validation worktree from 0.7.0-rc.63 to the unique local version.
  2. Keep mavenLocal() first in settings.gradle.kts; it is already configured correctly.
  3. Run dependency insight for devDebugRuntimeClasspath with refreshed dependencies and require it to resolve the unique com.synonym:ldk-node-android version.
  4. Run the focused host-side lifecycle tests:
    • LightningServiceTest
    • LightningNodeServiceTest
  5. Build assembleDevDebug only as a fast diagnostic artifact, then build assembleDevRelease as the device-validation artifact.
    • Install and exercise devRelease, not devDebug, so Kotlin/JNI packaging and runtime behavior match the release build type.
    • If the production release keystore is unavailable locally, sign the generated devRelease APK with the repository's debug keystore after assembly. Do not change the build type or source configuration.
  6. Extract the packaged lib/arm64-v8a/libldk_node.so from the devRelease APK and require its Build ID and SHA-256 to match the Maven Local AAR. This is the provenance gate proving the tested app contains PR fix: sigabrt runtime teardown #111.

Exercise the real Android lifecycle

  1. Prefer an arm64 Android 16 physical device matching the production reports. Use an arm64 emulator when no physical device is available and record that limitation.
  2. Install the locally signed devRelease APK, initialize a regtest wallet, and wait until the LDK node is running and synchronized against the local test backend.
  3. Open LDK Debug and automate the existing Restart action. Each iteration must wait for Node restarted successfully before starting the next one.
  4. Run 10 consecutive stop/destroy/start cycles with stable Electrum connectivity, including the first manual cycle and nine repetitions. Do not open channels or invoke splice APIs.
  5. Capture filtered adb logcat and the app's private logs for the full run.
  6. Fail immediately and preserve evidence if any cycle produces:
    • Cannot drop a runtime in a context where blocking is not allowed.
    • A Tokio 1.x context was found, but it is being shutdown.
    • SIGABRT, a native tombstone, or an unexpected app-process restart.
    • Node handle release still pending, NodeReleaseTimeout, or a restart that exceeds the existing release bound.
    • A second node starting before the previous native handle is released.

Repository checks

  • Run just compile, just test, and just lint with the local dependency selected.
  • Keep the ldk-node focused Rust/UniFFI/runtime tests from PR fix: sigabrt runtime teardown #111 as the native-layer gate.
  • Do not use the unrelated splicing integration suite as evidence for this fix.

Acceptance and evidence

  • Dependency insight resolves the unique Maven Local coordinate.
  • The AAR, debug symbols, and tested devRelease APK contain matching libldk_node.so Build IDs.
  • All focused lifecycle tests and repository checks pass.
  • Every device cycle completes within the existing release bounds.
  • Both production Tokio panic signatures remain absent.
  • No native abort, tombstone, stale-node poll, overlapping node lifetime, or release timeout occurs.
  • The evidence bundle contains the PR SHA, local coordinate, device/API/ABI, Build IDs, commands, test results, logcat, and app logs.

Cleanup and reporting

  • Restore gradle/libs.versions.toml after validation and require the Bitkit validation worktree to be clean.
  • Keep the uniquely versioned Maven Local artifact until review completes; it can be removed later without affecting released coordinates.
  • Keep this PR's Validation section synchronized with the recorded Android result, including the exact app build, device, cycle count, and Build ID.
  • Do not publish a new ldk-node version or permanently update Bitkit Android until this local gate passes.

@ovitrifovitrif changed the title fix: make runtime teardown deterministicfix: sigabrt runtime teardownSep 1, 2026
@coreyphillips

Copy link
Copy Markdown
Collaborator

Thanks for tackling this. I think three lifecycle gaps still need to be addressed before merging:

  • Exported payment and liquidity handles can pass the running check, race Node teardown, and call RuntimeControl::block_on after the owned runtime has shut down. I reproduced the Tokio context is being shutdown panic.
  • The timeout branches at src/runtime.rs:272 and src/runtime.rs:310 detach live background or event processing work while allowing stop() to succeed. A restart can then overlap the old generation against the same state and store.
  • ConfirmGate holds its mutex across external Confirm callbacks. begin_shutdown() can block forever behind persistence, and a callback panic poisons the gate and causes shutdown to panic.

Could we close these gaps and add focused regression coverage before merging?

ovitrif

This comment was marked as outdated.

Keep exported handles from using a shutting-down runtime, refuse
restart while detached work is still live, and stop Electrum confirm
gating from blocking or panicking shutdown.
@ovitrif

Copy link
Copy Markdown
CollaboratorAuthor

Addressed the three lifecycle gaps in b5c0b55:

  1. Exported payment/liquidity block_on now takes a runtime-use read lock. After Runtime drop, try_block_on returns NotRunning instead of panicking on a shutting-down Tokio handle. Drop waits for in-flight handle calls before shutdown_background().
  2. Timed-out background and event-processor work is retained. start() drains that leftover generation before admitting new tasks, and returns AlreadyRunning if it is still live.
  3. ConfirmGate is an AtomicBool. begin_shutdown() no longer waits on confirm callbacks or panics if a callback poisons a mutex.

Focused runtime and Electrum regressions cover the three cases.

@ovitrif
ovitrif merged commit 5c0c56b into mainSep 2, 2026
2 checks passed
@ovitrif
ovitrif deleted the codex/fix-runtime-shutdown branch September 2, 2026 17:15
Sign up for freeto 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.

fix: make owned runtime shutdown deterministic

2 participants

@ovitrif@coreyphillips
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix: sigabrt runtime teardown - #111

Merged
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown
Sep 2, 2026
Merged

fix: sigabrt runtime teardown#111
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown

Conversation

@ovitrif

@ovitrifovitrif commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes#110

Summary

  • Make Node the sole owner of the Tokio runtime and give tasks and exported child handles non-owning runtime control.
  • Close task admission, gate Electrum callbacks, and drain or cancel tracked work before runtime disposal.
  • Isolate VSS I/O on a store runtime whose fallback disposal is safe from async contexts.
  • Propagate event-processor persistence failures after cleanup and remove shutdown panic paths.
  • Bump all package surfaces to 0.7.0-rc.66 and regenerate Android, JVM, Python, and Swift artifacts without changing the public or UniFFI APIs.

Validation

  • Local verification: cargo fmt --all -- --check, RUSTFLAGS='-D warnings' cargo build, cargo check --release --features uniffi, and cargo test --features uniffi --lib (107 passed); focused runtime, Electrum, VSS, and release-mode regressions passed.
  • ./bindgen.sh regenerated JVM, Python, Swift, and Android artifacts for 0.7.0-rc.66; package builds, Maven Local publication, Swift checksum, native debug-symbol matching, and 16 KiB alignment checks passed.
  • Android Maven Local validation: the signed bitkit-dev-release-188-universal.apk from devRelease completed 100 consecutive in-app LDK node restarts on an arm64-v8a Android 16/API 36 emulator without SIGABRT or Tokio runtime-shutdown failures.

Release

@ovitrifovitrif self-assigned this Sep 1, 2026
@chatgpt-codex-connector

This comment has been minimized.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif

ovitrif commented Sep 1, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Validate ldk-node PR #111 in Bitkit Android

Objective

Validate the exact native library from synonymdev/ldk-node PR #111 inside a Bitkit Android build before publishing another ldk-node package.

The validation must exercise Bitkit's real node stop, destroy, and rebuild paths. It does not need the ldk-node channel_full_cycle integration test or any splicing flow.

Recorded Android validation

  • ldk-node head: dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Maven Local coordinate: com.synonym:ldk-node-android:0.7.0-rc.65-pr111.dc37559.
  • App build: signed bitkit-dev-release-188-universal.apk from the devRelease build type.
  • Device: arm64-v8a Android 16/API 36 emulator.
  • Lifecycle result: 100 consecutive in-app LDK Debug node restarts completed without SIGABRT or Tokio runtime-shutdown failures.
  • Native provenance: the APK's arm64-v8a libldk_node.so Build ID was 20b0b1bc8e28be7e153b5265e3c3f094c37ae6fa, matching the Maven Local AAR.

Inputs and isolation

  • Use the latest reviewed PR fix: sigabrt runtime teardown #111 head and record its full commit SHA before building. The current head is dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Derive a unique Maven Local version from that head, for example 0.7.0-rc.65-pr111.dc37559, so the artifact cannot be confused with a released rc.65 package.
  • Use a temporary Bitkit Android validation worktree based on the intended app baseline. Keep the existing checkout and unrelated work unchanged.
  • Save commands, dependency evidence, Build IDs, device details, and logs under .ai/logs/<YYYY-MM-DD>/ldk-pr111-android/.

Build and publish the local ldk-node artifact

  1. Confirm the ldk-node worktree is clean and its checked-out SHA is the recorded PR head.
  2. Run scripts/uniffi_bindgen_generate_kotlin_android.sh with ORG_GRADLE_PROJECT_version set to the unique local version.
    • The existing script builds release-smaller native libraries for armeabi-v7a, arm64-v8a, and x86_64.
    • It generates the Kotlin bindings and native debug-symbol archive.
    • It validates stripped libraries and 16 KB alignment.
    • It publishes com.synonym:ldk-node-android:<localVersion> to Maven Local.
  3. Verify Maven Local contains the AAR, POM, sources, Javadoc, and native-debug-symbols artifact for the unique coordinate.
  4. Record the Build ID and SHA-256 of each ABI's libldk_node.so from the AAR and debug-symbol archive.

Consume the artifact from Bitkit Android

  1. Change only gradle/libs.versions.toml in the validation worktree from 0.7.0-rc.63 to the unique local version.
  2. Keep mavenLocal() first in settings.gradle.kts; it is already configured correctly.
  3. Run dependency insight for devDebugRuntimeClasspath with refreshed dependencies and require it to resolve the unique com.synonym:ldk-node-android version.
  4. Run the focused host-side lifecycle tests:
    • LightningServiceTest
    • LightningNodeServiceTest
  5. Build assembleDevDebug only as a fast diagnostic artifact, then build assembleDevRelease as the device-validation artifact.
    • Install and exercise devRelease, not devDebug, so Kotlin/JNI packaging and runtime behavior match the release build type.
    • If the production release keystore is unavailable locally, sign the generated devRelease APK with the repository's debug keystore after assembly. Do not change the build type or source configuration.
  6. Extract the packaged lib/arm64-v8a/libldk_node.so from the devRelease APK and require its Build ID and SHA-256 to match the Maven Local AAR. This is the provenance gate proving the tested app contains PR fix: sigabrt runtime teardown #111.

Exercise the real Android lifecycle

  1. Prefer an arm64 Android 16 physical device matching the production reports. Use an arm64 emulator when no physical device is available and record that limitation.
  2. Install the locally signed devRelease APK, initialize a regtest wallet, and wait until the LDK node is running and synchronized against the local test backend.
  3. Open LDK Debug and automate the existing Restart action. Each iteration must wait for Node restarted successfully before starting the next one.
  4. Run 10 consecutive stop/destroy/start cycles with stable Electrum connectivity, including the first manual cycle and nine repetitions. Do not open channels or invoke splice APIs.
  5. Capture filtered adb logcat and the app's private logs for the full run.
  6. Fail immediately and preserve evidence if any cycle produces:
    • Cannot drop a runtime in a context where blocking is not allowed.
    • A Tokio 1.x context was found, but it is being shutdown.
    • SIGABRT, a native tombstone, or an unexpected app-process restart.
    • Node handle release still pending, NodeReleaseTimeout, or a restart that exceeds the existing release bound.
    • A second node starting before the previous native handle is released.

Repository checks

  • Run just compile, just test, and just lint with the local dependency selected.
  • Keep the ldk-node focused Rust/UniFFI/runtime tests from PR fix: sigabrt runtime teardown #111 as the native-layer gate.
  • Do not use the unrelated splicing integration suite as evidence for this fix.

Acceptance and evidence

  • Dependency insight resolves the unique Maven Local coordinate.
  • The AAR, debug symbols, and tested devRelease APK contain matching libldk_node.so Build IDs.
  • All focused lifecycle tests and repository checks pass.
  • Every device cycle completes within the existing release bounds.
  • Both production Tokio panic signatures remain absent.
  • No native abort, tombstone, stale-node poll, overlapping node lifetime, or release timeout occurs.
  • The evidence bundle contains the PR SHA, local coordinate, device/API/ABI, Build IDs, commands, test results, logcat, and app logs.

Cleanup and reporting

  • Restore gradle/libs.versions.toml after validation and require the Bitkit validation worktree to be clean.
  • Keep the uniquely versioned Maven Local artifact until review completes; it can be removed later without affecting released coordinates.
  • Keep this PR's Validation section synchronized with the recorded Android result, including the exact app build, device, cycle count, and Build ID.
  • Do not publish a new ldk-node version or permanently update Bitkit Android until this local gate passes.

@ovitrifovitrif changed the title fix: make runtime teardown deterministicfix: sigabrt runtime teardownSep 1, 2026
@coreyphillips

Copy link
Copy Markdown
Collaborator

Thanks for tackling this. I think three lifecycle gaps still need to be addressed before merging:

  • Exported payment and liquidity handles can pass the running check, race Node teardown, and call RuntimeControl::block_on after the owned runtime has shut down. I reproduced the Tokio context is being shutdown panic.
  • The timeout branches at src/runtime.rs:272 and src/runtime.rs:310 detach live background or event processing work while allowing stop() to succeed. A restart can then overlap the old generation against the same state and store.
  • ConfirmGate holds its mutex across external Confirm callbacks. begin_shutdown() can block forever behind persistence, and a callback panic poisons the gate and causes shutdown to panic.

Could we close these gaps and add focused regression coverage before merging?

ovitrif

This comment was marked as outdated.

Keep exported handles from using a shutting-down runtime, refuse
restart while detached work is still live, and stop Electrum confirm
gating from blocking or panicking shutdown.
@ovitrif

Copy link
Copy Markdown
CollaboratorAuthor

Addressed the three lifecycle gaps in b5c0b55:

  1. Exported payment/liquidity block_on now takes a runtime-use read lock. After Runtime drop, try_block_on returns NotRunning instead of panicking on a shutting-down Tokio handle. Drop waits for in-flight handle calls before shutdown_background().
  2. Timed-out background and event-processor work is retained. start() drains that leftover generation before admitting new tasks, and returns AlreadyRunning if it is still live.
  3. ConfirmGate is an AtomicBool. begin_shutdown() no longer waits on confirm callbacks or panics if a callback poisons a mutex.

Focused runtime and Electrum regressions cover the three cases.

@ovitrif
ovitrif merged commit 5c0c56b into mainSep 2, 2026
2 checks passed
@ovitrif
ovitrif deleted the codex/fix-runtime-shutdown branch September 2, 2026 17:15
Sign up for freeto 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.

fix: make owned runtime shutdown deterministic

2 participants

@ovitrif@coreyphillips
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: sigabrt runtime teardown - #111

Merged
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown
Sep 2, 2026
Merged

fix: sigabrt runtime teardown#111
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown

Conversation

@ovitrif

@ovitrifovitrif commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes#110

Summary

  • Make Node the sole owner of the Tokio runtime and give tasks and exported child handles non-owning runtime control.
  • Close task admission, gate Electrum callbacks, and drain or cancel tracked work before runtime disposal.
  • Isolate VSS I/O on a store runtime whose fallback disposal is safe from async contexts.
  • Propagate event-processor persistence failures after cleanup and remove shutdown panic paths.
  • Bump all package surfaces to 0.7.0-rc.66 and regenerate Android, JVM, Python, and Swift artifacts without changing the public or UniFFI APIs.

Validation

  • Local verification: cargo fmt --all -- --check, RUSTFLAGS='-D warnings' cargo build, cargo check --release --features uniffi, and cargo test --features uniffi --lib (107 passed); focused runtime, Electrum, VSS, and release-mode regressions passed.
  • ./bindgen.sh regenerated JVM, Python, Swift, and Android artifacts for 0.7.0-rc.66; package builds, Maven Local publication, Swift checksum, native debug-symbol matching, and 16 KiB alignment checks passed.
  • Android Maven Local validation: the signed bitkit-dev-release-188-universal.apk from devRelease completed 100 consecutive in-app LDK node restarts on an arm64-v8a Android 16/API 36 emulator without SIGABRT or Tokio runtime-shutdown failures.

Release

@ovitrifovitrif self-assigned this Sep 1, 2026
@chatgpt-codex-connector

This comment has been minimized.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif

ovitrif commented Sep 1, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Validate ldk-node PR #111 in Bitkit Android

Objective

Validate the exact native library from synonymdev/ldk-node PR #111 inside a Bitkit Android build before publishing another ldk-node package.

The validation must exercise Bitkit's real node stop, destroy, and rebuild paths. It does not need the ldk-node channel_full_cycle integration test or any splicing flow.

Recorded Android validation

  • ldk-node head: dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Maven Local coordinate: com.synonym:ldk-node-android:0.7.0-rc.65-pr111.dc37559.
  • App build: signed bitkit-dev-release-188-universal.apk from the devRelease build type.
  • Device: arm64-v8a Android 16/API 36 emulator.
  • Lifecycle result: 100 consecutive in-app LDK Debug node restarts completed without SIGABRT or Tokio runtime-shutdown failures.
  • Native provenance: the APK's arm64-v8a libldk_node.so Build ID was 20b0b1bc8e28be7e153b5265e3c3f094c37ae6fa, matching the Maven Local AAR.

Inputs and isolation

  • Use the latest reviewed PR fix: sigabrt runtime teardown #111 head and record its full commit SHA before building. The current head is dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Derive a unique Maven Local version from that head, for example 0.7.0-rc.65-pr111.dc37559, so the artifact cannot be confused with a released rc.65 package.
  • Use a temporary Bitkit Android validation worktree based on the intended app baseline. Keep the existing checkout and unrelated work unchanged.
  • Save commands, dependency evidence, Build IDs, device details, and logs under .ai/logs/<YYYY-MM-DD>/ldk-pr111-android/.

Build and publish the local ldk-node artifact

  1. Confirm the ldk-node worktree is clean and its checked-out SHA is the recorded PR head.
  2. Run scripts/uniffi_bindgen_generate_kotlin_android.sh with ORG_GRADLE_PROJECT_version set to the unique local version.
    • The existing script builds release-smaller native libraries for armeabi-v7a, arm64-v8a, and x86_64.
    • It generates the Kotlin bindings and native debug-symbol archive.
    • It validates stripped libraries and 16 KB alignment.
    • It publishes com.synonym:ldk-node-android:<localVersion> to Maven Local.
  3. Verify Maven Local contains the AAR, POM, sources, Javadoc, and native-debug-symbols artifact for the unique coordinate.
  4. Record the Build ID and SHA-256 of each ABI's libldk_node.so from the AAR and debug-symbol archive.

Consume the artifact from Bitkit Android

  1. Change only gradle/libs.versions.toml in the validation worktree from 0.7.0-rc.63 to the unique local version.
  2. Keep mavenLocal() first in settings.gradle.kts; it is already configured correctly.
  3. Run dependency insight for devDebugRuntimeClasspath with refreshed dependencies and require it to resolve the unique com.synonym:ldk-node-android version.
  4. Run the focused host-side lifecycle tests:
    • LightningServiceTest
    • LightningNodeServiceTest
  5. Build assembleDevDebug only as a fast diagnostic artifact, then build assembleDevRelease as the device-validation artifact.
    • Install and exercise devRelease, not devDebug, so Kotlin/JNI packaging and runtime behavior match the release build type.
    • If the production release keystore is unavailable locally, sign the generated devRelease APK with the repository's debug keystore after assembly. Do not change the build type or source configuration.
  6. Extract the packaged lib/arm64-v8a/libldk_node.so from the devRelease APK and require its Build ID and SHA-256 to match the Maven Local AAR. This is the provenance gate proving the tested app contains PR fix: sigabrt runtime teardown #111.

Exercise the real Android lifecycle

  1. Prefer an arm64 Android 16 physical device matching the production reports. Use an arm64 emulator when no physical device is available and record that limitation.
  2. Install the locally signed devRelease APK, initialize a regtest wallet, and wait until the LDK node is running and synchronized against the local test backend.
  3. Open LDK Debug and automate the existing Restart action. Each iteration must wait for Node restarted successfully before starting the next one.
  4. Run 10 consecutive stop/destroy/start cycles with stable Electrum connectivity, including the first manual cycle and nine repetitions. Do not open channels or invoke splice APIs.
  5. Capture filtered adb logcat and the app's private logs for the full run.
  6. Fail immediately and preserve evidence if any cycle produces:
    • Cannot drop a runtime in a context where blocking is not allowed.
    • A Tokio 1.x context was found, but it is being shutdown.
    • SIGABRT, a native tombstone, or an unexpected app-process restart.
    • Node handle release still pending, NodeReleaseTimeout, or a restart that exceeds the existing release bound.
    • A second node starting before the previous native handle is released.

Repository checks

  • Run just compile, just test, and just lint with the local dependency selected.
  • Keep the ldk-node focused Rust/UniFFI/runtime tests from PR fix: sigabrt runtime teardown #111 as the native-layer gate.
  • Do not use the unrelated splicing integration suite as evidence for this fix.

Acceptance and evidence

  • Dependency insight resolves the unique Maven Local coordinate.
  • The AAR, debug symbols, and tested devRelease APK contain matching libldk_node.so Build IDs.
  • All focused lifecycle tests and repository checks pass.
  • Every device cycle completes within the existing release bounds.
  • Both production Tokio panic signatures remain absent.
  • No native abort, tombstone, stale-node poll, overlapping node lifetime, or release timeout occurs.
  • The evidence bundle contains the PR SHA, local coordinate, device/API/ABI, Build IDs, commands, test results, logcat, and app logs.

Cleanup and reporting

  • Restore gradle/libs.versions.toml after validation and require the Bitkit validation worktree to be clean.
  • Keep the uniquely versioned Maven Local artifact until review completes; it can be removed later without affecting released coordinates.
  • Keep this PR's Validation section synchronized with the recorded Android result, including the exact app build, device, cycle count, and Build ID.
  • Do not publish a new ldk-node version or permanently update Bitkit Android until this local gate passes.

@ovitrifovitrif changed the title fix: make runtime teardown deterministicfix: sigabrt runtime teardownSep 1, 2026
@coreyphillips

Copy link
Copy Markdown
Collaborator

Thanks for tackling this. I think three lifecycle gaps still need to be addressed before merging:

  • Exported payment and liquidity handles can pass the running check, race Node teardown, and call RuntimeControl::block_on after the owned runtime has shut down. I reproduced the Tokio context is being shutdown panic.
  • The timeout branches at src/runtime.rs:272 and src/runtime.rs:310 detach live background or event processing work while allowing stop() to succeed. A restart can then overlap the old generation against the same state and store.
  • ConfirmGate holds its mutex across external Confirm callbacks. begin_shutdown() can block forever behind persistence, and a callback panic poisons the gate and causes shutdown to panic.

Could we close these gaps and add focused regression coverage before merging?

ovitrif

This comment was marked as outdated.

Keep exported handles from using a shutting-down runtime, refuse
restart while detached work is still live, and stop Electrum confirm
gating from blocking or panicking shutdown.
@ovitrif

Copy link
Copy Markdown
CollaboratorAuthor

Addressed the three lifecycle gaps in b5c0b55:

  1. Exported payment/liquidity block_on now takes a runtime-use read lock. After Runtime drop, try_block_on returns NotRunning instead of panicking on a shutting-down Tokio handle. Drop waits for in-flight handle calls before shutdown_background().
  2. Timed-out background and event-processor work is retained. start() drains that leftover generation before admitting new tasks, and returns AlreadyRunning if it is still live.
  3. ConfirmGate is an AtomicBool. begin_shutdown() no longer waits on confirm callbacks or panics if a callback poisons a mutex.

Focused runtime and Electrum regressions cover the three cases.

@ovitrif
ovitrif merged commit 5c0c56b into mainSep 2, 2026
2 checks passed
@ovitrif
ovitrif deleted the codex/fix-runtime-shutdown branch September 2, 2026 17:15
Sign up for freeto 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.

fix: make owned runtime shutdown deterministic

2 participants

@ovitrif@coreyphillips
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: sigabrt runtime teardown - #111

Merged
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown
Sep 2, 2026
Merged

fix: sigabrt runtime teardown#111
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown

Conversation

@ovitrif

@ovitrifovitrif commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes#110

Summary

  • Make Node the sole owner of the Tokio runtime and give tasks and exported child handles non-owning runtime control.
  • Close task admission, gate Electrum callbacks, and drain or cancel tracked work before runtime disposal.
  • Isolate VSS I/O on a store runtime whose fallback disposal is safe from async contexts.
  • Propagate event-processor persistence failures after cleanup and remove shutdown panic paths.
  • Bump all package surfaces to 0.7.0-rc.66 and regenerate Android, JVM, Python, and Swift artifacts without changing the public or UniFFI APIs.

Validation

  • Local verification: cargo fmt --all -- --check, RUSTFLAGS='-D warnings' cargo build, cargo check --release --features uniffi, and cargo test --features uniffi --lib (107 passed); focused runtime, Electrum, VSS, and release-mode regressions passed.
  • ./bindgen.sh regenerated JVM, Python, Swift, and Android artifacts for 0.7.0-rc.66; package builds, Maven Local publication, Swift checksum, native debug-symbol matching, and 16 KiB alignment checks passed.
  • Android Maven Local validation: the signed bitkit-dev-release-188-universal.apk from devRelease completed 100 consecutive in-app LDK node restarts on an arm64-v8a Android 16/API 36 emulator without SIGABRT or Tokio runtime-shutdown failures.

Release

@ovitrifovitrif self-assigned this Sep 1, 2026
@chatgpt-codex-connector

This comment has been minimized.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif

ovitrif commented Sep 1, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Validate ldk-node PR #111 in Bitkit Android

Objective

Validate the exact native library from synonymdev/ldk-node PR #111 inside a Bitkit Android build before publishing another ldk-node package.

The validation must exercise Bitkit's real node stop, destroy, and rebuild paths. It does not need the ldk-node channel_full_cycle integration test or any splicing flow.

Recorded Android validation

  • ldk-node head: dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Maven Local coordinate: com.synonym:ldk-node-android:0.7.0-rc.65-pr111.dc37559.
  • App build: signed bitkit-dev-release-188-universal.apk from the devRelease build type.
  • Device: arm64-v8a Android 16/API 36 emulator.
  • Lifecycle result: 100 consecutive in-app LDK Debug node restarts completed without SIGABRT or Tokio runtime-shutdown failures.
  • Native provenance: the APK's arm64-v8a libldk_node.so Build ID was 20b0b1bc8e28be7e153b5265e3c3f094c37ae6fa, matching the Maven Local AAR.

Inputs and isolation

  • Use the latest reviewed PR fix: sigabrt runtime teardown #111 head and record its full commit SHA before building. The current head is dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Derive a unique Maven Local version from that head, for example 0.7.0-rc.65-pr111.dc37559, so the artifact cannot be confused with a released rc.65 package.
  • Use a temporary Bitkit Android validation worktree based on the intended app baseline. Keep the existing checkout and unrelated work unchanged.
  • Save commands, dependency evidence, Build IDs, device details, and logs under .ai/logs/<YYYY-MM-DD>/ldk-pr111-android/.

Build and publish the local ldk-node artifact

  1. Confirm the ldk-node worktree is clean and its checked-out SHA is the recorded PR head.
  2. Run scripts/uniffi_bindgen_generate_kotlin_android.sh with ORG_GRADLE_PROJECT_version set to the unique local version.
    • The existing script builds release-smaller native libraries for armeabi-v7a, arm64-v8a, and x86_64.
    • It generates the Kotlin bindings and native debug-symbol archive.
    • It validates stripped libraries and 16 KB alignment.
    • It publishes com.synonym:ldk-node-android:<localVersion> to Maven Local.
  3. Verify Maven Local contains the AAR, POM, sources, Javadoc, and native-debug-symbols artifact for the unique coordinate.
  4. Record the Build ID and SHA-256 of each ABI's libldk_node.so from the AAR and debug-symbol archive.

Consume the artifact from Bitkit Android

  1. Change only gradle/libs.versions.toml in the validation worktree from 0.7.0-rc.63 to the unique local version.
  2. Keep mavenLocal() first in settings.gradle.kts; it is already configured correctly.
  3. Run dependency insight for devDebugRuntimeClasspath with refreshed dependencies and require it to resolve the unique com.synonym:ldk-node-android version.
  4. Run the focused host-side lifecycle tests:
    • LightningServiceTest
    • LightningNodeServiceTest
  5. Build assembleDevDebug only as a fast diagnostic artifact, then build assembleDevRelease as the device-validation artifact.
    • Install and exercise devRelease, not devDebug, so Kotlin/JNI packaging and runtime behavior match the release build type.
    • If the production release keystore is unavailable locally, sign the generated devRelease APK with the repository's debug keystore after assembly. Do not change the build type or source configuration.
  6. Extract the packaged lib/arm64-v8a/libldk_node.so from the devRelease APK and require its Build ID and SHA-256 to match the Maven Local AAR. This is the provenance gate proving the tested app contains PR fix: sigabrt runtime teardown #111.

Exercise the real Android lifecycle

  1. Prefer an arm64 Android 16 physical device matching the production reports. Use an arm64 emulator when no physical device is available and record that limitation.
  2. Install the locally signed devRelease APK, initialize a regtest wallet, and wait until the LDK node is running and synchronized against the local test backend.
  3. Open LDK Debug and automate the existing Restart action. Each iteration must wait for Node restarted successfully before starting the next one.
  4. Run 10 consecutive stop/destroy/start cycles with stable Electrum connectivity, including the first manual cycle and nine repetitions. Do not open channels or invoke splice APIs.
  5. Capture filtered adb logcat and the app's private logs for the full run.
  6. Fail immediately and preserve evidence if any cycle produces:
    • Cannot drop a runtime in a context where blocking is not allowed.
    • A Tokio 1.x context was found, but it is being shutdown.
    • SIGABRT, a native tombstone, or an unexpected app-process restart.
    • Node handle release still pending, NodeReleaseTimeout, or a restart that exceeds the existing release bound.
    • A second node starting before the previous native handle is released.

Repository checks

  • Run just compile, just test, and just lint with the local dependency selected.
  • Keep the ldk-node focused Rust/UniFFI/runtime tests from PR fix: sigabrt runtime teardown #111 as the native-layer gate.
  • Do not use the unrelated splicing integration suite as evidence for this fix.

Acceptance and evidence

  • Dependency insight resolves the unique Maven Local coordinate.
  • The AAR, debug symbols, and tested devRelease APK contain matching libldk_node.so Build IDs.
  • All focused lifecycle tests and repository checks pass.
  • Every device cycle completes within the existing release bounds.
  • Both production Tokio panic signatures remain absent.
  • No native abort, tombstone, stale-node poll, overlapping node lifetime, or release timeout occurs.
  • The evidence bundle contains the PR SHA, local coordinate, device/API/ABI, Build IDs, commands, test results, logcat, and app logs.

Cleanup and reporting

  • Restore gradle/libs.versions.toml after validation and require the Bitkit validation worktree to be clean.
  • Keep the uniquely versioned Maven Local artifact until review completes; it can be removed later without affecting released coordinates.
  • Keep this PR's Validation section synchronized with the recorded Android result, including the exact app build, device, cycle count, and Build ID.
  • Do not publish a new ldk-node version or permanently update Bitkit Android until this local gate passes.

@ovitrifovitrif changed the title fix: make runtime teardown deterministicfix: sigabrt runtime teardownSep 1, 2026
@coreyphillips

Copy link
Copy Markdown
Collaborator

Thanks for tackling this. I think three lifecycle gaps still need to be addressed before merging:

  • Exported payment and liquidity handles can pass the running check, race Node teardown, and call RuntimeControl::block_on after the owned runtime has shut down. I reproduced the Tokio context is being shutdown panic.
  • The timeout branches at src/runtime.rs:272 and src/runtime.rs:310 detach live background or event processing work while allowing stop() to succeed. A restart can then overlap the old generation against the same state and store.
  • ConfirmGate holds its mutex across external Confirm callbacks. begin_shutdown() can block forever behind persistence, and a callback panic poisons the gate and causes shutdown to panic.

Could we close these gaps and add focused regression coverage before merging?

ovitrif

This comment was marked as outdated.

Keep exported handles from using a shutting-down runtime, refuse
restart while detached work is still live, and stop Electrum confirm
gating from blocking or panicking shutdown.
@ovitrif

Copy link
Copy Markdown
CollaboratorAuthor

Addressed the three lifecycle gaps in b5c0b55:

  1. Exported payment/liquidity block_on now takes a runtime-use read lock. After Runtime drop, try_block_on returns NotRunning instead of panicking on a shutting-down Tokio handle. Drop waits for in-flight handle calls before shutdown_background().
  2. Timed-out background and event-processor work is retained. start() drains that leftover generation before admitting new tasks, and returns AlreadyRunning if it is still live.
  3. ConfirmGate is an AtomicBool. begin_shutdown() no longer waits on confirm callbacks or panics if a callback poisons a mutex.

Focused runtime and Electrum regressions cover the three cases.

@ovitrif
ovitrif merged commit 5c0c56b into mainSep 2, 2026
2 checks passed
@ovitrif
ovitrif deleted the codex/fix-runtime-shutdown branch September 2, 2026 17:15
Sign up for freeto 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.

fix: make owned runtime shutdown deterministic

2 participants

@ovitrif@coreyphillips
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix: sigabrt runtime teardown - #111

Merged
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown
Sep 2, 2026
Merged

fix: sigabrt runtime teardown#111
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown

Conversation

@ovitrif

@ovitrifovitrif commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes#110

Summary

  • Make Node the sole owner of the Tokio runtime and give tasks and exported child handles non-owning runtime control.
  • Close task admission, gate Electrum callbacks, and drain or cancel tracked work before runtime disposal.
  • Isolate VSS I/O on a store runtime whose fallback disposal is safe from async contexts.
  • Propagate event-processor persistence failures after cleanup and remove shutdown panic paths.
  • Bump all package surfaces to 0.7.0-rc.66 and regenerate Android, JVM, Python, and Swift artifacts without changing the public or UniFFI APIs.

Validation

  • Local verification: cargo fmt --all -- --check, RUSTFLAGS='-D warnings' cargo build, cargo check --release --features uniffi, and cargo test --features uniffi --lib (107 passed); focused runtime, Electrum, VSS, and release-mode regressions passed.
  • ./bindgen.sh regenerated JVM, Python, Swift, and Android artifacts for 0.7.0-rc.66; package builds, Maven Local publication, Swift checksum, native debug-symbol matching, and 16 KiB alignment checks passed.
  • Android Maven Local validation: the signed bitkit-dev-release-188-universal.apk from devRelease completed 100 consecutive in-app LDK node restarts on an arm64-v8a Android 16/API 36 emulator without SIGABRT or Tokio runtime-shutdown failures.

Release

@ovitrifovitrif self-assigned this Sep 1, 2026
@chatgpt-codex-connector

This comment has been minimized.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif

ovitrif commented Sep 1, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Validate ldk-node PR #111 in Bitkit Android

Objective

Validate the exact native library from synonymdev/ldk-node PR #111 inside a Bitkit Android build before publishing another ldk-node package.

The validation must exercise Bitkit's real node stop, destroy, and rebuild paths. It does not need the ldk-node channel_full_cycle integration test or any splicing flow.

Recorded Android validation

  • ldk-node head: dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Maven Local coordinate: com.synonym:ldk-node-android:0.7.0-rc.65-pr111.dc37559.
  • App build: signed bitkit-dev-release-188-universal.apk from the devRelease build type.
  • Device: arm64-v8a Android 16/API 36 emulator.
  • Lifecycle result: 100 consecutive in-app LDK Debug node restarts completed without SIGABRT or Tokio runtime-shutdown failures.
  • Native provenance: the APK's arm64-v8a libldk_node.so Build ID was 20b0b1bc8e28be7e153b5265e3c3f094c37ae6fa, matching the Maven Local AAR.

Inputs and isolation

  • Use the latest reviewed PR fix: sigabrt runtime teardown #111 head and record its full commit SHA before building. The current head is dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Derive a unique Maven Local version from that head, for example 0.7.0-rc.65-pr111.dc37559, so the artifact cannot be confused with a released rc.65 package.
  • Use a temporary Bitkit Android validation worktree based on the intended app baseline. Keep the existing checkout and unrelated work unchanged.
  • Save commands, dependency evidence, Build IDs, device details, and logs under .ai/logs/<YYYY-MM-DD>/ldk-pr111-android/.

Build and publish the local ldk-node artifact

  1. Confirm the ldk-node worktree is clean and its checked-out SHA is the recorded PR head.
  2. Run scripts/uniffi_bindgen_generate_kotlin_android.sh with ORG_GRADLE_PROJECT_version set to the unique local version.
    • The existing script builds release-smaller native libraries for armeabi-v7a, arm64-v8a, and x86_64.
    • It generates the Kotlin bindings and native debug-symbol archive.
    • It validates stripped libraries and 16 KB alignment.
    • It publishes com.synonym:ldk-node-android:<localVersion> to Maven Local.
  3. Verify Maven Local contains the AAR, POM, sources, Javadoc, and native-debug-symbols artifact for the unique coordinate.
  4. Record the Build ID and SHA-256 of each ABI's libldk_node.so from the AAR and debug-symbol archive.

Consume the artifact from Bitkit Android

  1. Change only gradle/libs.versions.toml in the validation worktree from 0.7.0-rc.63 to the unique local version.
  2. Keep mavenLocal() first in settings.gradle.kts; it is already configured correctly.
  3. Run dependency insight for devDebugRuntimeClasspath with refreshed dependencies and require it to resolve the unique com.synonym:ldk-node-android version.
  4. Run the focused host-side lifecycle tests:
    • LightningServiceTest
    • LightningNodeServiceTest
  5. Build assembleDevDebug only as a fast diagnostic artifact, then build assembleDevRelease as the device-validation artifact.
    • Install and exercise devRelease, not devDebug, so Kotlin/JNI packaging and runtime behavior match the release build type.
    • If the production release keystore is unavailable locally, sign the generated devRelease APK with the repository's debug keystore after assembly. Do not change the build type or source configuration.
  6. Extract the packaged lib/arm64-v8a/libldk_node.so from the devRelease APK and require its Build ID and SHA-256 to match the Maven Local AAR. This is the provenance gate proving the tested app contains PR fix: sigabrt runtime teardown #111.

Exercise the real Android lifecycle

  1. Prefer an arm64 Android 16 physical device matching the production reports. Use an arm64 emulator when no physical device is available and record that limitation.
  2. Install the locally signed devRelease APK, initialize a regtest wallet, and wait until the LDK node is running and synchronized against the local test backend.
  3. Open LDK Debug and automate the existing Restart action. Each iteration must wait for Node restarted successfully before starting the next one.
  4. Run 10 consecutive stop/destroy/start cycles with stable Electrum connectivity, including the first manual cycle and nine repetitions. Do not open channels or invoke splice APIs.
  5. Capture filtered adb logcat and the app's private logs for the full run.
  6. Fail immediately and preserve evidence if any cycle produces:
    • Cannot drop a runtime in a context where blocking is not allowed.
    • A Tokio 1.x context was found, but it is being shutdown.
    • SIGABRT, a native tombstone, or an unexpected app-process restart.
    • Node handle release still pending, NodeReleaseTimeout, or a restart that exceeds the existing release bound.
    • A second node starting before the previous native handle is released.

Repository checks

  • Run just compile, just test, and just lint with the local dependency selected.
  • Keep the ldk-node focused Rust/UniFFI/runtime tests from PR fix: sigabrt runtime teardown #111 as the native-layer gate.
  • Do not use the unrelated splicing integration suite as evidence for this fix.

Acceptance and evidence

  • Dependency insight resolves the unique Maven Local coordinate.
  • The AAR, debug symbols, and tested devRelease APK contain matching libldk_node.so Build IDs.
  • All focused lifecycle tests and repository checks pass.
  • Every device cycle completes within the existing release bounds.
  • Both production Tokio panic signatures remain absent.
  • No native abort, tombstone, stale-node poll, overlapping node lifetime, or release timeout occurs.
  • The evidence bundle contains the PR SHA, local coordinate, device/API/ABI, Build IDs, commands, test results, logcat, and app logs.

Cleanup and reporting

  • Restore gradle/libs.versions.toml after validation and require the Bitkit validation worktree to be clean.
  • Keep the uniquely versioned Maven Local artifact until review completes; it can be removed later without affecting released coordinates.
  • Keep this PR's Validation section synchronized with the recorded Android result, including the exact app build, device, cycle count, and Build ID.
  • Do not publish a new ldk-node version or permanently update Bitkit Android until this local gate passes.

@ovitrifovitrif changed the title fix: make runtime teardown deterministicfix: sigabrt runtime teardownSep 1, 2026
@coreyphillips

Copy link
Copy Markdown
Collaborator

Thanks for tackling this. I think three lifecycle gaps still need to be addressed before merging:

  • Exported payment and liquidity handles can pass the running check, race Node teardown, and call RuntimeControl::block_on after the owned runtime has shut down. I reproduced the Tokio context is being shutdown panic.
  • The timeout branches at src/runtime.rs:272 and src/runtime.rs:310 detach live background or event processing work while allowing stop() to succeed. A restart can then overlap the old generation against the same state and store.
  • ConfirmGate holds its mutex across external Confirm callbacks. begin_shutdown() can block forever behind persistence, and a callback panic poisons the gate and causes shutdown to panic.

Could we close these gaps and add focused regression coverage before merging?

ovitrif

This comment was marked as outdated.

Keep exported handles from using a shutting-down runtime, refuse
restart while detached work is still live, and stop Electrum confirm
gating from blocking or panicking shutdown.
@ovitrif

Copy link
Copy Markdown
CollaboratorAuthor

Addressed the three lifecycle gaps in b5c0b55:

  1. Exported payment/liquidity block_on now takes a runtime-use read lock. After Runtime drop, try_block_on returns NotRunning instead of panicking on a shutting-down Tokio handle. Drop waits for in-flight handle calls before shutdown_background().
  2. Timed-out background and event-processor work is retained. start() drains that leftover generation before admitting new tasks, and returns AlreadyRunning if it is still live.
  3. ConfirmGate is an AtomicBool. begin_shutdown() no longer waits on confirm callbacks or panics if a callback poisons a mutex.

Focused runtime and Electrum regressions cover the three cases.

@ovitrif
ovitrif merged commit 5c0c56b into mainSep 2, 2026
2 checks passed
@ovitrif
ovitrif deleted the codex/fix-runtime-shutdown branch September 2, 2026 17:15
Sign up for freeto 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.

fix: make owned runtime shutdown deterministic

2 participants

@ovitrif@coreyphillips
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: sigabrt runtime teardown - #111

Merged
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown
Sep 2, 2026
Merged

fix: sigabrt runtime teardown#111
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown

Conversation

@ovitrif

@ovitrifovitrif commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes#110

Summary

  • Make Node the sole owner of the Tokio runtime and give tasks and exported child handles non-owning runtime control.
  • Close task admission, gate Electrum callbacks, and drain or cancel tracked work before runtime disposal.
  • Isolate VSS I/O on a store runtime whose fallback disposal is safe from async contexts.
  • Propagate event-processor persistence failures after cleanup and remove shutdown panic paths.
  • Bump all package surfaces to 0.7.0-rc.66 and regenerate Android, JVM, Python, and Swift artifacts without changing the public or UniFFI APIs.

Validation

  • Local verification: cargo fmt --all -- --check, RUSTFLAGS='-D warnings' cargo build, cargo check --release --features uniffi, and cargo test --features uniffi --lib (107 passed); focused runtime, Electrum, VSS, and release-mode regressions passed.
  • ./bindgen.sh regenerated JVM, Python, Swift, and Android artifacts for 0.7.0-rc.66; package builds, Maven Local publication, Swift checksum, native debug-symbol matching, and 16 KiB alignment checks passed.
  • Android Maven Local validation: the signed bitkit-dev-release-188-universal.apk from devRelease completed 100 consecutive in-app LDK node restarts on an arm64-v8a Android 16/API 36 emulator without SIGABRT or Tokio runtime-shutdown failures.

Release

@ovitrifovitrif self-assigned this Sep 1, 2026
@chatgpt-codex-connector

This comment has been minimized.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif

ovitrif commented Sep 1, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Validate ldk-node PR #111 in Bitkit Android

Objective

Validate the exact native library from synonymdev/ldk-node PR #111 inside a Bitkit Android build before publishing another ldk-node package.

The validation must exercise Bitkit's real node stop, destroy, and rebuild paths. It does not need the ldk-node channel_full_cycle integration test or any splicing flow.

Recorded Android validation

  • ldk-node head: dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Maven Local coordinate: com.synonym:ldk-node-android:0.7.0-rc.65-pr111.dc37559.
  • App build: signed bitkit-dev-release-188-universal.apk from the devRelease build type.
  • Device: arm64-v8a Android 16/API 36 emulator.
  • Lifecycle result: 100 consecutive in-app LDK Debug node restarts completed without SIGABRT or Tokio runtime-shutdown failures.
  • Native provenance: the APK's arm64-v8a libldk_node.so Build ID was 20b0b1bc8e28be7e153b5265e3c3f094c37ae6fa, matching the Maven Local AAR.

Inputs and isolation

  • Use the latest reviewed PR fix: sigabrt runtime teardown #111 head and record its full commit SHA before building. The current head is dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Derive a unique Maven Local version from that head, for example 0.7.0-rc.65-pr111.dc37559, so the artifact cannot be confused with a released rc.65 package.
  • Use a temporary Bitkit Android validation worktree based on the intended app baseline. Keep the existing checkout and unrelated work unchanged.
  • Save commands, dependency evidence, Build IDs, device details, and logs under .ai/logs/<YYYY-MM-DD>/ldk-pr111-android/.

Build and publish the local ldk-node artifact

  1. Confirm the ldk-node worktree is clean and its checked-out SHA is the recorded PR head.
  2. Run scripts/uniffi_bindgen_generate_kotlin_android.sh with ORG_GRADLE_PROJECT_version set to the unique local version.
    • The existing script builds release-smaller native libraries for armeabi-v7a, arm64-v8a, and x86_64.
    • It generates the Kotlin bindings and native debug-symbol archive.
    • It validates stripped libraries and 16 KB alignment.
    • It publishes com.synonym:ldk-node-android:<localVersion> to Maven Local.
  3. Verify Maven Local contains the AAR, POM, sources, Javadoc, and native-debug-symbols artifact for the unique coordinate.
  4. Record the Build ID and SHA-256 of each ABI's libldk_node.so from the AAR and debug-symbol archive.

Consume the artifact from Bitkit Android

  1. Change only gradle/libs.versions.toml in the validation worktree from 0.7.0-rc.63 to the unique local version.
  2. Keep mavenLocal() first in settings.gradle.kts; it is already configured correctly.
  3. Run dependency insight for devDebugRuntimeClasspath with refreshed dependencies and require it to resolve the unique com.synonym:ldk-node-android version.
  4. Run the focused host-side lifecycle tests:
    • LightningServiceTest
    • LightningNodeServiceTest
  5. Build assembleDevDebug only as a fast diagnostic artifact, then build assembleDevRelease as the device-validation artifact.
    • Install and exercise devRelease, not devDebug, so Kotlin/JNI packaging and runtime behavior match the release build type.
    • If the production release keystore is unavailable locally, sign the generated devRelease APK with the repository's debug keystore after assembly. Do not change the build type or source configuration.
  6. Extract the packaged lib/arm64-v8a/libldk_node.so from the devRelease APK and require its Build ID and SHA-256 to match the Maven Local AAR. This is the provenance gate proving the tested app contains PR fix: sigabrt runtime teardown #111.

Exercise the real Android lifecycle

  1. Prefer an arm64 Android 16 physical device matching the production reports. Use an arm64 emulator when no physical device is available and record that limitation.
  2. Install the locally signed devRelease APK, initialize a regtest wallet, and wait until the LDK node is running and synchronized against the local test backend.
  3. Open LDK Debug and automate the existing Restart action. Each iteration must wait for Node restarted successfully before starting the next one.
  4. Run 10 consecutive stop/destroy/start cycles with stable Electrum connectivity, including the first manual cycle and nine repetitions. Do not open channels or invoke splice APIs.
  5. Capture filtered adb logcat and the app's private logs for the full run.
  6. Fail immediately and preserve evidence if any cycle produces:
    • Cannot drop a runtime in a context where blocking is not allowed.
    • A Tokio 1.x context was found, but it is being shutdown.
    • SIGABRT, a native tombstone, or an unexpected app-process restart.
    • Node handle release still pending, NodeReleaseTimeout, or a restart that exceeds the existing release bound.
    • A second node starting before the previous native handle is released.

Repository checks

  • Run just compile, just test, and just lint with the local dependency selected.
  • Keep the ldk-node focused Rust/UniFFI/runtime tests from PR fix: sigabrt runtime teardown #111 as the native-layer gate.
  • Do not use the unrelated splicing integration suite as evidence for this fix.

Acceptance and evidence

  • Dependency insight resolves the unique Maven Local coordinate.
  • The AAR, debug symbols, and tested devRelease APK contain matching libldk_node.so Build IDs.
  • All focused lifecycle tests and repository checks pass.
  • Every device cycle completes within the existing release bounds.
  • Both production Tokio panic signatures remain absent.
  • No native abort, tombstone, stale-node poll, overlapping node lifetime, or release timeout occurs.
  • The evidence bundle contains the PR SHA, local coordinate, device/API/ABI, Build IDs, commands, test results, logcat, and app logs.

Cleanup and reporting

  • Restore gradle/libs.versions.toml after validation and require the Bitkit validation worktree to be clean.
  • Keep the uniquely versioned Maven Local artifact until review completes; it can be removed later without affecting released coordinates.
  • Keep this PR's Validation section synchronized with the recorded Android result, including the exact app build, device, cycle count, and Build ID.
  • Do not publish a new ldk-node version or permanently update Bitkit Android until this local gate passes.

@ovitrifovitrif changed the title fix: make runtime teardown deterministicfix: sigabrt runtime teardownSep 1, 2026
@coreyphillips

Copy link
Copy Markdown
Collaborator

Thanks for tackling this. I think three lifecycle gaps still need to be addressed before merging:

  • Exported payment and liquidity handles can pass the running check, race Node teardown, and call RuntimeControl::block_on after the owned runtime has shut down. I reproduced the Tokio context is being shutdown panic.
  • The timeout branches at src/runtime.rs:272 and src/runtime.rs:310 detach live background or event processing work while allowing stop() to succeed. A restart can then overlap the old generation against the same state and store.
  • ConfirmGate holds its mutex across external Confirm callbacks. begin_shutdown() can block forever behind persistence, and a callback panic poisons the gate and causes shutdown to panic.

Could we close these gaps and add focused regression coverage before merging?

ovitrif

This comment was marked as outdated.

Keep exported handles from using a shutting-down runtime, refuse
restart while detached work is still live, and stop Electrum confirm
gating from blocking or panicking shutdown.
@ovitrif

Copy link
Copy Markdown
CollaboratorAuthor

Addressed the three lifecycle gaps in b5c0b55:

  1. Exported payment/liquidity block_on now takes a runtime-use read lock. After Runtime drop, try_block_on returns NotRunning instead of panicking on a shutting-down Tokio handle. Drop waits for in-flight handle calls before shutdown_background().
  2. Timed-out background and event-processor work is retained. start() drains that leftover generation before admitting new tasks, and returns AlreadyRunning if it is still live.
  3. ConfirmGate is an AtomicBool. begin_shutdown() no longer waits on confirm callbacks or panics if a callback poisons a mutex.

Focused runtime and Electrum regressions cover the three cases.

@ovitrif
ovitrif merged commit 5c0c56b into mainSep 2, 2026
2 checks passed
@ovitrif
ovitrif deleted the codex/fix-runtime-shutdown branch September 2, 2026 17:15
Sign up for freeto 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.

fix: make owned runtime shutdown deterministic

2 participants

@ovitrif@coreyphillips
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: sigabrt runtime teardown - #111

Merged
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown
Sep 2, 2026
Merged

fix: sigabrt runtime teardown#111
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown

Conversation

@ovitrif

@ovitrifovitrif commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes#110

Summary

  • Make Node the sole owner of the Tokio runtime and give tasks and exported child handles non-owning runtime control.
  • Close task admission, gate Electrum callbacks, and drain or cancel tracked work before runtime disposal.
  • Isolate VSS I/O on a store runtime whose fallback disposal is safe from async contexts.
  • Propagate event-processor persistence failures after cleanup and remove shutdown panic paths.
  • Bump all package surfaces to 0.7.0-rc.66 and regenerate Android, JVM, Python, and Swift artifacts without changing the public or UniFFI APIs.

Validation

  • Local verification: cargo fmt --all -- --check, RUSTFLAGS='-D warnings' cargo build, cargo check --release --features uniffi, and cargo test --features uniffi --lib (107 passed); focused runtime, Electrum, VSS, and release-mode regressions passed.
  • ./bindgen.sh regenerated JVM, Python, Swift, and Android artifacts for 0.7.0-rc.66; package builds, Maven Local publication, Swift checksum, native debug-symbol matching, and 16 KiB alignment checks passed.
  • Android Maven Local validation: the signed bitkit-dev-release-188-universal.apk from devRelease completed 100 consecutive in-app LDK node restarts on an arm64-v8a Android 16/API 36 emulator without SIGABRT or Tokio runtime-shutdown failures.

Release

@ovitrifovitrif self-assigned this Sep 1, 2026
@chatgpt-codex-connector

This comment has been minimized.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif

ovitrif commented Sep 1, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Validate ldk-node PR #111 in Bitkit Android

Objective

Validate the exact native library from synonymdev/ldk-node PR #111 inside a Bitkit Android build before publishing another ldk-node package.

The validation must exercise Bitkit's real node stop, destroy, and rebuild paths. It does not need the ldk-node channel_full_cycle integration test or any splicing flow.

Recorded Android validation

  • ldk-node head: dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Maven Local coordinate: com.synonym:ldk-node-android:0.7.0-rc.65-pr111.dc37559.
  • App build: signed bitkit-dev-release-188-universal.apk from the devRelease build type.
  • Device: arm64-v8a Android 16/API 36 emulator.
  • Lifecycle result: 100 consecutive in-app LDK Debug node restarts completed without SIGABRT or Tokio runtime-shutdown failures.
  • Native provenance: the APK's arm64-v8a libldk_node.so Build ID was 20b0b1bc8e28be7e153b5265e3c3f094c37ae6fa, matching the Maven Local AAR.

Inputs and isolation

  • Use the latest reviewed PR fix: sigabrt runtime teardown #111 head and record its full commit SHA before building. The current head is dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Derive a unique Maven Local version from that head, for example 0.7.0-rc.65-pr111.dc37559, so the artifact cannot be confused with a released rc.65 package.
  • Use a temporary Bitkit Android validation worktree based on the intended app baseline. Keep the existing checkout and unrelated work unchanged.
  • Save commands, dependency evidence, Build IDs, device details, and logs under .ai/logs/<YYYY-MM-DD>/ldk-pr111-android/.

Build and publish the local ldk-node artifact

  1. Confirm the ldk-node worktree is clean and its checked-out SHA is the recorded PR head.
  2. Run scripts/uniffi_bindgen_generate_kotlin_android.sh with ORG_GRADLE_PROJECT_version set to the unique local version.
    • The existing script builds release-smaller native libraries for armeabi-v7a, arm64-v8a, and x86_64.
    • It generates the Kotlin bindings and native debug-symbol archive.
    • It validates stripped libraries and 16 KB alignment.
    • It publishes com.synonym:ldk-node-android:<localVersion> to Maven Local.
  3. Verify Maven Local contains the AAR, POM, sources, Javadoc, and native-debug-symbols artifact for the unique coordinate.
  4. Record the Build ID and SHA-256 of each ABI's libldk_node.so from the AAR and debug-symbol archive.

Consume the artifact from Bitkit Android

  1. Change only gradle/libs.versions.toml in the validation worktree from 0.7.0-rc.63 to the unique local version.
  2. Keep mavenLocal() first in settings.gradle.kts; it is already configured correctly.
  3. Run dependency insight for devDebugRuntimeClasspath with refreshed dependencies and require it to resolve the unique com.synonym:ldk-node-android version.
  4. Run the focused host-side lifecycle tests:
    • LightningServiceTest
    • LightningNodeServiceTest
  5. Build assembleDevDebug only as a fast diagnostic artifact, then build assembleDevRelease as the device-validation artifact.
    • Install and exercise devRelease, not devDebug, so Kotlin/JNI packaging and runtime behavior match the release build type.
    • If the production release keystore is unavailable locally, sign the generated devRelease APK with the repository's debug keystore after assembly. Do not change the build type or source configuration.
  6. Extract the packaged lib/arm64-v8a/libldk_node.so from the devRelease APK and require its Build ID and SHA-256 to match the Maven Local AAR. This is the provenance gate proving the tested app contains PR fix: sigabrt runtime teardown #111.

Exercise the real Android lifecycle

  1. Prefer an arm64 Android 16 physical device matching the production reports. Use an arm64 emulator when no physical device is available and record that limitation.
  2. Install the locally signed devRelease APK, initialize a regtest wallet, and wait until the LDK node is running and synchronized against the local test backend.
  3. Open LDK Debug and automate the existing Restart action. Each iteration must wait for Node restarted successfully before starting the next one.
  4. Run 10 consecutive stop/destroy/start cycles with stable Electrum connectivity, including the first manual cycle and nine repetitions. Do not open channels or invoke splice APIs.
  5. Capture filtered adb logcat and the app's private logs for the full run.
  6. Fail immediately and preserve evidence if any cycle produces:
    • Cannot drop a runtime in a context where blocking is not allowed.
    • A Tokio 1.x context was found, but it is being shutdown.
    • SIGABRT, a native tombstone, or an unexpected app-process restart.
    • Node handle release still pending, NodeReleaseTimeout, or a restart that exceeds the existing release bound.
    • A second node starting before the previous native handle is released.

Repository checks

  • Run just compile, just test, and just lint with the local dependency selected.
  • Keep the ldk-node focused Rust/UniFFI/runtime tests from PR fix: sigabrt runtime teardown #111 as the native-layer gate.
  • Do not use the unrelated splicing integration suite as evidence for this fix.

Acceptance and evidence

  • Dependency insight resolves the unique Maven Local coordinate.
  • The AAR, debug symbols, and tested devRelease APK contain matching libldk_node.so Build IDs.
  • All focused lifecycle tests and repository checks pass.
  • Every device cycle completes within the existing release bounds.
  • Both production Tokio panic signatures remain absent.
  • No native abort, tombstone, stale-node poll, overlapping node lifetime, or release timeout occurs.
  • The evidence bundle contains the PR SHA, local coordinate, device/API/ABI, Build IDs, commands, test results, logcat, and app logs.

Cleanup and reporting

  • Restore gradle/libs.versions.toml after validation and require the Bitkit validation worktree to be clean.
  • Keep the uniquely versioned Maven Local artifact until review completes; it can be removed later without affecting released coordinates.
  • Keep this PR's Validation section synchronized with the recorded Android result, including the exact app build, device, cycle count, and Build ID.
  • Do not publish a new ldk-node version or permanently update Bitkit Android until this local gate passes.

@ovitrifovitrif changed the title fix: make runtime teardown deterministicfix: sigabrt runtime teardownSep 1, 2026
@coreyphillips

Copy link
Copy Markdown
Collaborator

Thanks for tackling this. I think three lifecycle gaps still need to be addressed before merging:

  • Exported payment and liquidity handles can pass the running check, race Node teardown, and call RuntimeControl::block_on after the owned runtime has shut down. I reproduced the Tokio context is being shutdown panic.
  • The timeout branches at src/runtime.rs:272 and src/runtime.rs:310 detach live background or event processing work while allowing stop() to succeed. A restart can then overlap the old generation against the same state and store.
  • ConfirmGate holds its mutex across external Confirm callbacks. begin_shutdown() can block forever behind persistence, and a callback panic poisons the gate and causes shutdown to panic.

Could we close these gaps and add focused regression coverage before merging?

ovitrif

This comment was marked as outdated.

Keep exported handles from using a shutting-down runtime, refuse
restart while detached work is still live, and stop Electrum confirm
gating from blocking or panicking shutdown.
@ovitrif

Copy link
Copy Markdown
CollaboratorAuthor

Addressed the three lifecycle gaps in b5c0b55:

  1. Exported payment/liquidity block_on now takes a runtime-use read lock. After Runtime drop, try_block_on returns NotRunning instead of panicking on a shutting-down Tokio handle. Drop waits for in-flight handle calls before shutdown_background().
  2. Timed-out background and event-processor work is retained. start() drains that leftover generation before admitting new tasks, and returns AlreadyRunning if it is still live.
  3. ConfirmGate is an AtomicBool. begin_shutdown() no longer waits on confirm callbacks or panics if a callback poisons a mutex.

Focused runtime and Electrum regressions cover the three cases.

@ovitrif
ovitrif merged commit 5c0c56b into mainSep 2, 2026
2 checks passed
@ovitrif
ovitrif deleted the codex/fix-runtime-shutdown branch September 2, 2026 17:15
Sign up for freeto 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.

fix: make owned runtime shutdown deterministic

2 participants

@ovitrif@coreyphillips
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix: sigabrt runtime teardown - #111

Merged
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown
Sep 2, 2026
Merged

fix: sigabrt runtime teardown#111
ovitrif merged 5 commits into
mainfrom
codex/fix-runtime-shutdown

Conversation

@ovitrif

@ovitrifovitrif commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes#110

Summary

  • Make Node the sole owner of the Tokio runtime and give tasks and exported child handles non-owning runtime control.
  • Close task admission, gate Electrum callbacks, and drain or cancel tracked work before runtime disposal.
  • Isolate VSS I/O on a store runtime whose fallback disposal is safe from async contexts.
  • Propagate event-processor persistence failures after cleanup and remove shutdown panic paths.
  • Bump all package surfaces to 0.7.0-rc.66 and regenerate Android, JVM, Python, and Swift artifacts without changing the public or UniFFI APIs.

Validation

  • Local verification: cargo fmt --all -- --check, RUSTFLAGS='-D warnings' cargo build, cargo check --release --features uniffi, and cargo test --features uniffi --lib (107 passed); focused runtime, Electrum, VSS, and release-mode regressions passed.
  • ./bindgen.sh regenerated JVM, Python, Swift, and Android artifacts for 0.7.0-rc.66; package builds, Maven Local publication, Swift checksum, native debug-symbol matching, and 16 KiB alignment checks passed.
  • Android Maven Local validation: the signed bitkit-dev-release-188-universal.apk from devRelease completed 100 consecutive in-app LDK node restarts on an arm64-v8a Android 16/API 36 emulator without SIGABRT or Tokio runtime-shutdown failures.

Release

@ovitrifovitrif self-assigned this Sep 1, 2026
@chatgpt-codex-connector

This comment has been minimized.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif

ovitrif commented Sep 1, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Validate ldk-node PR #111 in Bitkit Android

Objective

Validate the exact native library from synonymdev/ldk-node PR #111 inside a Bitkit Android build before publishing another ldk-node package.

The validation must exercise Bitkit's real node stop, destroy, and rebuild paths. It does not need the ldk-node channel_full_cycle integration test or any splicing flow.

Recorded Android validation

  • ldk-node head: dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Maven Local coordinate: com.synonym:ldk-node-android:0.7.0-rc.65-pr111.dc37559.
  • App build: signed bitkit-dev-release-188-universal.apk from the devRelease build type.
  • Device: arm64-v8a Android 16/API 36 emulator.
  • Lifecycle result: 100 consecutive in-app LDK Debug node restarts completed without SIGABRT or Tokio runtime-shutdown failures.
  • Native provenance: the APK's arm64-v8a libldk_node.so Build ID was 20b0b1bc8e28be7e153b5265e3c3f094c37ae6fa, matching the Maven Local AAR.

Inputs and isolation

  • Use the latest reviewed PR fix: sigabrt runtime teardown #111 head and record its full commit SHA before building. The current head is dc37559a9eb6c68bf2ce1ed81b49eb58e25e0fe3.
  • Derive a unique Maven Local version from that head, for example 0.7.0-rc.65-pr111.dc37559, so the artifact cannot be confused with a released rc.65 package.
  • Use a temporary Bitkit Android validation worktree based on the intended app baseline. Keep the existing checkout and unrelated work unchanged.
  • Save commands, dependency evidence, Build IDs, device details, and logs under .ai/logs/<YYYY-MM-DD>/ldk-pr111-android/.

Build and publish the local ldk-node artifact

  1. Confirm the ldk-node worktree is clean and its checked-out SHA is the recorded PR head.
  2. Run scripts/uniffi_bindgen_generate_kotlin_android.sh with ORG_GRADLE_PROJECT_version set to the unique local version.
    • The existing script builds release-smaller native libraries for armeabi-v7a, arm64-v8a, and x86_64.
    • It generates the Kotlin bindings and native debug-symbol archive.
    • It validates stripped libraries and 16 KB alignment.
    • It publishes com.synonym:ldk-node-android:<localVersion> to Maven Local.
  3. Verify Maven Local contains the AAR, POM, sources, Javadoc, and native-debug-symbols artifact for the unique coordinate.
  4. Record the Build ID and SHA-256 of each ABI's libldk_node.so from the AAR and debug-symbol archive.

Consume the artifact from Bitkit Android

  1. Change only gradle/libs.versions.toml in the validation worktree from 0.7.0-rc.63 to the unique local version.
  2. Keep mavenLocal() first in settings.gradle.kts; it is already configured correctly.
  3. Run dependency insight for devDebugRuntimeClasspath with refreshed dependencies and require it to resolve the unique com.synonym:ldk-node-android version.
  4. Run the focused host-side lifecycle tests:
    • LightningServiceTest
    • LightningNodeServiceTest
  5. Build assembleDevDebug only as a fast diagnostic artifact, then build assembleDevRelease as the device-validation artifact.
    • Install and exercise devRelease, not devDebug, so Kotlin/JNI packaging and runtime behavior match the release build type.
    • If the production release keystore is unavailable locally, sign the generated devRelease APK with the repository's debug keystore after assembly. Do not change the build type or source configuration.
  6. Extract the packaged lib/arm64-v8a/libldk_node.so from the devRelease APK and require its Build ID and SHA-256 to match the Maven Local AAR. This is the provenance gate proving the tested app contains PR fix: sigabrt runtime teardown #111.

Exercise the real Android lifecycle

  1. Prefer an arm64 Android 16 physical device matching the production reports. Use an arm64 emulator when no physical device is available and record that limitation.
  2. Install the locally signed devRelease APK, initialize a regtest wallet, and wait until the LDK node is running and synchronized against the local test backend.
  3. Open LDK Debug and automate the existing Restart action. Each iteration must wait for Node restarted successfully before starting the next one.
  4. Run 10 consecutive stop/destroy/start cycles with stable Electrum connectivity, including the first manual cycle and nine repetitions. Do not open channels or invoke splice APIs.
  5. Capture filtered adb logcat and the app's private logs for the full run.
  6. Fail immediately and preserve evidence if any cycle produces:
    • Cannot drop a runtime in a context where blocking is not allowed.
    • A Tokio 1.x context was found, but it is being shutdown.
    • SIGABRT, a native tombstone, or an unexpected app-process restart.
    • Node handle release still pending, NodeReleaseTimeout, or a restart that exceeds the existing release bound.
    • A second node starting before the previous native handle is released.

Repository checks

  • Run just compile, just test, and just lint with the local dependency selected.
  • Keep the ldk-node focused Rust/UniFFI/runtime tests from PR fix: sigabrt runtime teardown #111 as the native-layer gate.
  • Do not use the unrelated splicing integration suite as evidence for this fix.

Acceptance and evidence

  • Dependency insight resolves the unique Maven Local coordinate.
  • The AAR, debug symbols, and tested devRelease APK contain matching libldk_node.so Build IDs.
  • All focused lifecycle tests and repository checks pass.
  • Every device cycle completes within the existing release bounds.
  • Both production Tokio panic signatures remain absent.
  • No native abort, tombstone, stale-node poll, overlapping node lifetime, or release timeout occurs.
  • The evidence bundle contains the PR SHA, local coordinate, device/API/ABI, Build IDs, commands, test results, logcat, and app logs.

Cleanup and reporting

  • Restore gradle/libs.versions.toml after validation and require the Bitkit validation worktree to be clean.
  • Keep the uniquely versioned Maven Local artifact until review completes; it can be removed later without affecting released coordinates.
  • Keep this PR's Validation section synchronized with the recorded Android result, including the exact app build, device, cycle count, and Build ID.
  • Do not publish a new ldk-node version or permanently update Bitkit Android until this local gate passes.

@ovitrifovitrif changed the title fix: make runtime teardown deterministicfix: sigabrt runtime teardownSep 1, 2026
@coreyphillips

Copy link
Copy Markdown
Collaborator

Thanks for tackling this. I think three lifecycle gaps still need to be addressed before merging:

  • Exported payment and liquidity handles can pass the running check, race Node teardown, and call RuntimeControl::block_on after the owned runtime has shut down. I reproduced the Tokio context is being shutdown panic.
  • The timeout branches at src/runtime.rs:272 and src/runtime.rs:310 detach live background or event processing work while allowing stop() to succeed. A restart can then overlap the old generation against the same state and store.
  • ConfirmGate holds its mutex across external Confirm callbacks. begin_shutdown() can block forever behind persistence, and a callback panic poisons the gate and causes shutdown to panic.

Could we close these gaps and add focused regression coverage before merging?

ovitrif

This comment was marked as outdated.

Keep exported handles from using a shutting-down runtime, refuse
restart while detached work is still live, and stop Electrum confirm
gating from blocking or panicking shutdown.
@ovitrif

Copy link
Copy Markdown
CollaboratorAuthor

Addressed the three lifecycle gaps in b5c0b55:

  1. Exported payment/liquidity block_on now takes a runtime-use read lock. After Runtime drop, try_block_on returns NotRunning instead of panicking on a shutting-down Tokio handle. Drop waits for in-flight handle calls before shutdown_background().
  2. Timed-out background and event-processor work is retained. start() drains that leftover generation before admitting new tasks, and returns AlreadyRunning if it is still live.
  3. ConfirmGate is an AtomicBool. begin_shutdown() no longer waits on confirm callbacks or panics if a callback poisons a mutex.

Focused runtime and Electrum regressions cover the three cases.

@ovitrif
ovitrif merged commit 5c0c56b into mainSep 2, 2026
2 checks passed
@ovitrif
ovitrif deleted the codex/fix-runtime-shutdown branch September 2, 2026 17:15
Sign up for freeto 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.

fix: make owned runtime shutdown deterministic

2 participants

@ovitrif@coreyphillips