Repository navigation
fix(monero-sys): join the wallet thread when the last handle drops - #1229
Merged
Merged
Conversation
The wallet thread was detached. When the last WalletHandle dropped, the thread closed the wallet in the background, and the process could exit while closeWallet was still running.
Deploying eigenwallet-docs with
|
| Latest commit: |
75733e3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://42f4257b.eigenwallet-docs.pages.dev |
| Branch Preview URL: | https://fix-monero-sys-join-wallet-t.eigenwallet-docs.pages.dev |
…join Move the call sender into the shared WalletThread. Its Drop closes the channel and then joins the thread, so the order no longer depends on the field declaration order of WalletHandle.
Author
|
This should fix the sporadically failing CI tests. |
Write the comments of WalletThread in Simplified Technical English and rename the thread argument of WalletHandle::new to wallet_thread.
std::thread::Builder::name panics on a NUL byte, and open_with makes the thread name from the wallet path. Return an error before the thread starts.
The C++ getBlockchainHeightByDate catches every exception, sets the wallet status error and returns 0. Check the wallet status when the height is 0.
…t thread panicked Move the code that reads the panic message into panic_message, and use it in Wallet::run and in WalletThread::drop.
Say which thread each comment means: the wallet thread, or the thread that drops the last WalletHandle. Rename the JoinHandle field and variable to join_handle, so that "handle" does not also mean the JoinHandle.
Updated the changelog to reflect recent fixes and improvements in GUI and CLI related to Monero wallet errors and transfer proofs.
Make the WalletHandle right after the wallet thread starts, before the first await. If the caller cancels open_with, the WalletHandle drops, and WalletThread::drop waits until the wallet thread stops.
Author
|
@Einliterflasche Please review. |
WalletHandleListener and TauriWalletListener held an Arc<WalletHandle> to the wallet that owns them. Thus the channel to the wallet thread never closed, and the main wallet never closed. Add WeakWalletHandle, and let the listeners upgrade it in their tokio tasks.
added 4 commits
October 9, 2026 20:55
Put back two lines in wallets.rs that rustfmt changed with no relation to this PR. Say that all calls from Rust to the C++ wallet run on the wallet thread, because the C++ refresh thread also uses the C++ wallet.
…ot end On a wallet thread and during a listener callback, WalletThread::drop now starts a new thread that waits for the wallet thread. Before, drop skipped the join on the wallet thread, and a drop in a listener callback on the C++ refresh thread deadlocked, because closeWallet waits for that thread.
…callback Only wallet threads and C++ threads run listener callbacks, and a C++ thread runs Rust code only in a callback. A true value can only stop drop from waiting, so the reset guard is not necessary.
forward_cpp_log is a second call from C++ into Rust, and the C++ refresh thread makes it too. Replace run_listener_callback with enter_cpp_callback, call it at the start of all nine entries, and document that nothing waits for the wallet-join thread.
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
1 participant
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.
WalletHandle::open_withdropped theJoinHandleof the wallet thread. When the last handle dropped, the thread closed the wallet in the background, and the process could exit duringcloseWallet. This could cause a SIGSEGV.WalletHandlenow keeps theJoinHandlein a sharedArc. When the last handle drops, it closes the call channel and then joins the thread. On a wallet thread and during a listener callback, a wait for the wallet thread cannot end. There, a new thread waits for the wallet thread in place ofdrop.sequenceDiagram participant C as Caller participant T as WalletThread (shared Arc) participant W as Wallet thread (Wallet::run) C->>T: drop last WalletHandle T->>T: call_sender.take() closes the channel W->>W: blocking_recv() returns None W->>W: dispose pending transactions W->>W: closeWallet (stores the wallet) T->>W: join() W-->>T: thread exits T-->>C: drop returns Note over C: The process can now exit safelyThis PR also fixes two small issues in
monero-sys/src/lib.rs:open_withmakes the thread name from the wallet path, andstd::thread::Builder::namepanics on a NUL byte.open_withnow returns an error for a path with a NUL byte.FfiWallet::get_blockchain_height_by_datereturnedOk(0)when the C++ lookup failed, because the C++ function catches all exceptions and returns 0. A CLI restore height by date then started a full rescan from the genesis block. The function now callscheck_error()when the height is 0 and returns the error.The PR also fixes a reference loop that kept the main wallet open.
WalletHandleListenerandTauriWalletListenerheld anArc<WalletHandle>to the wallet that they listen to, and the wallet thread owns its listeners. Thus the channel to the wallet thread never closed, and the main wallet inWalletsnever closed and never did a final store. The PR addsWeakWalletHandle(WalletHandle::downgradeandWeakWalletHandle::upgrade). Both listeners now hold aWeakWalletHandleand upgrade it in their tokio tasks.