Repository navigation
fix: keyring calls panicked when made from inside the async runtime - #34
Merged
Merged
Conversation
`mikrotui host migrate` aborted with "Cannot start a runtime from within a runtime" as soon as there was actually a password to migrate. The Secret Service backend bridges its async D-Bus client into the synchronous keyring API with `block_on`, and tokio refuses that on one of its worker threads. `main` is `#[tokio::main]`, so every keyring call was on one. The reasoning that put it there was mine and it was wrong: the `rt-tokio-crypto-rust` feature was chosen to "reuse the Tokio runtime MikroTUI already has", but what that feature does is make the backend *use* tokio, and using tokio's block_on from inside tokio is exactly what panics. Migrate is only where it surfaced. Ten call sites reach the keyring, and all of them run inside the runtime: the `host` subcommands, password resolution at startup, and `App::switch_host` — which would have taken a live TUI session down mid-use, on a keypress, with a router already connected. Keyring work now runs on a thread of its own. That is done inside the backend rather than at each call site, so a new caller cannot forget it, and it holds whichever store feature is selected instead of depending on one backend's internals. A thread per call is affordable: these are rare and already wait on D-Bus. Reproducing it needed a config that actually has an obfuscated password — without one, migrate returns before touching the keyring, which is why the existing tests never saw it. The regression test calls all three entry points from inside `#[tokio::test]`, and fails with the original panic when the fix is reverted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported from real use. The Secret Service backend bridges its async D-Bus client into the synchronous keyring API with
block_on, and tokio refuses that on one of its worker threads.mainis#[tokio::main], so every keyring call was on one.My reasoning was wrong
I picked the
rt-tokio-crypto-rustfeature to "reuse the Tokio runtime MikroTUI already has". What that feature actually does is make the backend use tokio — and calling tokio'sblock_onfrom inside tokio is precisely what panics.Migrate is only where it surfaced
Ten call sites reach the keyring, all inside the runtime:
hostsubcommandsApp::switch_host— this one would have killed a live TUI session on a keypress, with a router already connectedThe fix
Keyring work runs on a thread of its own, done inside the backend rather than at each call site so a new caller cannot forget it — and so it holds whichever store feature is selected, instead of depending on one backend's internals. A thread per call is affordable: these are rare and already wait on D-Bus.
Why the tests missed it
Reproducing needs a config that actually has an obfuscated password — without one,
migratereturns before touching the keyring. My first two attempts to reproduce printed "nothing to migrate" and looked fine.The regression test calls all three entry points from inside
#[tokio::test]. Reverted, it fails with the original panic:Verified end to end afterwards:
host migratemoved a password andhost listreported🔐 keyring.106 tests.
🤖 Generated with Claude Code