Make GUI updates non-blocking and avoid redraw artifacts - #2146
Conversation
Embedded lianad exposes async GUI methods, but the underlying DaemonControl API is synchronous and may block on database work, backend RPC, poller coordination, or daemon shutdown. Move those calls through tokio::task::spawn_blocking and keep the existing single-command serialization with a std mutex. This prevents synchronous embedded daemon work from occupying async executor workers while preserving the previous access semantics.
The loader's async sync helper used std::thread::sleep, which blocks a runtime worker even though the only intent is to delay a follow-up daemon info request. Use tokio::time::sleep so the task yields during the delay. This keeps the async runtime responsive while preserving the existing one-second wait behavior.
Menu navigation for preselected transactions and PSBTs used Handle::block_on to query the daemon before changing panels. With an embedded daemon this waited synchronously on GUI update handling, making navigation sensitive to daemon latency. Route those lookups through Task::perform and apply the preselection when the result arrives. If the item is unavailable or stale, fall back to the normal panel reload path without blocking the UI update.
Saving node settings stopped the current daemon, started a replacement, and wrote daemon.toml directly from the app update path. That path used Handle::block_on around daemon.stop(), so slow shutdown or startup could freeze GUI message handling. Run the reload as an iced task instead. The old daemon is stopped asynchronously, the blocking embedded daemon startup and config write run on the blocking pool, and the app swaps in the new daemon only after the task succeeds.
Spend form updates recalculated draft transactions with Handle::block_on around daemon create_spend_tx/create_recovery calls. With an embedded daemon, backend latency in those calls could block iced update processing while the user edits recipients, feerate, or coin selection. Move redrafting into Task::perform and carry the previous result data back through an explicit RedraftSpend message. A monotonically increasing request id makes stale daemon results harmless, so rapid edits cannot apply an older auto-selection or max-recipient estimate over newer form state.
A failed install tried to remove partially-created wallet data with Handle::block_on from the installer update path. That made the GUI wait for filesystem cleanup and settings/cache updates before it could render the installation error. Report the install failure to the current step immediately and schedule delete_failed_install as an iced task. The cleanup result is routed back only for logging, preserving the best-effort cleanup while keeping installer message handling responsive.
The launcher opened the delete-wallet modal by synchronously checking the user's Liana Connect membership, and confirmed deletion by synchronously awaiting delete_wallet from the modal update path. Both operations can involve network and filesystem work, so they could freeze launcher interactions. Open the modal immediately, run membership lookup and deletion as iced tasks, and apply results only when they still match the wallet shown by the modal. The message boundary uses a small local role enum instead of carrying the upstream UserRole type, keeping launcher messages cloneable and debuggable.
Remote-backend app startup updated daemon settings with Handle::block_on when the backend wallet alias differed from the local settings alias. That put an async settings-file write on the synchronous app-construction path. Create the app immediately and batch the alias update with the app's startup task. The new app message only logs failures, preserving the previous best-effort behavior without blocking the transition into the wallet UI.
The app and loader close hooks still used Handle::block_on to stop an embedded daemon, then synchronously stopped the managed bitcoind. Close handling therefore still had a sync wait on lianad shutdown, which is exactly the boundary this cleanup is trying to avoid. Schedule shutdown on the runtime instead. The task preserves the existing order by awaiting daemon.stop() first and then running bitcoind.stop() on the blocking pool, while the GUI close path no longer blocks on either operation.
Focused text inputs request periodic redraws for cursor blinking. Inputs inside card::simple could trigger a renderer artifact where the card's semi-transparent shadow was blended repeatedly, making the surrounding card darker on every blink. Make card::simple draw the regular card surface and border without a shadow. This keeps the shared card styling used by Send, Wallet alias, Blockchain rescan, and other form cards, while removing the translucent primitive from text-input redraw paths.
|
concept NACK, the rationale seems AI bullshit to me, and I dont see what problem it solves |
|
The concrete issue I see is that, when lianad is embedded in the GUI, the GUI exposes async daemon calls but Examples:
So the PR is a boundary fix: keep lianad synchronous, but move these potentially blocking embedded-daemon operations off the GUI async/update path. |
After hundread hours stress-testing the GUI, I never experienced this, we need a reproducible user-facing problem before a fix. Fixing potential bugs is a good way to introduce REAL bugs. |
|
@pythcoiner You are right, changes like this should be handled carefully and only when the bug is proven. I think my PR's scope is too large, but some fixes are useful. I reproduced the following scenario. When bitcoind stops, embedded lianad enters its synchronous bitcoind retry loop. It logs I also added a repro script for this scenario, and I'm attaching both the script and the video: close-freeze-demo.mp4The script: reproduce-gui-close-freeze.sh PS. The video was recorded by AI, the scenario was found by AI. I think this is an interesting way to find GUI glitches: let AI run the app in a virtual X server and look for bugs. It seems better to burn AI time than human time on this kind of issue hunting. |
Well my understanding is that, the "bug" is an edge case, that cant happend during a normal usage
I totaly disagree on this when it's about, easy to find, UI glitches, that do not impact users in real world. Our bottleneck is litteraly human time, and honnestly, this burn himan time.
PS: please if you want to share scripts, just inline them in the discussion. |
|
I think it is still a normal situation: stop bitcoind / electrs and then try to close liana-gui and it gets stuck and becomes not responding. IMHO the expected behavior is a normal shutdown in this situation. Maybe I'm over-engineering here. I tried to add another backend (BIP157) and got GUI stuck in many situations. I propose to close this PR, unless you find some pieces are useful. For example, this one looks useful for me: 99e22d4 ( |
No, its absolutely not a normal situation, it's an edge case:
I agree we should handle this more gracefully, but the solution I'd go is something like #672.
Thanks for looking at this, but I think (in respect of your time), the approach should be discussed in an issue before spending much time on works that have large scope like this: there are many undocumented details that can make such integration hard to get merged. |
2 participants
Summary
liana-gui/src/daemon/embedded.rs, so synchronous lianad work no longer occupies async executor workers directly.Handle::block_oncalls withTask::performor background runtime tasks acrossliana-gui/src/app/mod.rs,liana-gui/src/app/state/spend/step.rs,liana-gui/src/installer/mod.rs,liana-gui/src/launcher.rs,liana-gui/src/gui/tab.rs, andliana-gui/src/loader.rs.liana-gui/src/app/message.rsandliana-gui/src/installer/message.rsso preselection, daemon config reloads, spend redrafts, failed-install cleanup, remote alias sync, and wallet deletion complete without blocking GUI message handling.liana-gui/src/loader.rs.card::simpleinliana-ui/src/theme/card.rs.Rationale
liana-guiis async, but embeddedlianadremains synchronous internally. Before this change, several GUI-facing async methods or update paths still crossed that boundary without an explicit blocking boundary, or synchronously waited for daemon/database/network work from GUI update handling. This could make navigation, spend editing, settings changes, installer cleanup, wallet deletion, startup, and close handling sensitive to daemon latency.This keeps
lianadsynchronous and limits the change to the GUI boundary: synchronous embedded daemon calls are isolated on the blocking pool, and GUI state transitions schedule work as iced tasks instead of waiting inline.