Kill monero-wallet-rpc child process when receiving SIGINT, SIGTERM o… #1434
Conversation
binarybaron
force-pushed
the
kill-monero-rpc-on-signal-receive
branch
from
b3cea64
to
d362a41
Compare
binarybaron
force-pushed
the
kill-monero-rpc-on-signal-receive
branch
2 times, most recently
from
1697f2e
to
8418e57
Compare
|
seems to break the windows build, is there a cross platform way to do the same? |
|
I'll look into it |
binarybaron
force-pushed
the
kill-monero-rpc-on-signal-receive
branch
from
8418e57
to
09ce8ed
Compare
|
I made it platform specific but we still need to test this on Windows. I'll do it in the next few days |
|
Our current Tokio version doesn't have support for signals on windows... We'll have to upgrade |
binarybaron
force-pushed
the
kill-monero-rpc-on-signal-receive
branch
from
4c9b84f
to
78049a9
Compare
…r SIGHUP When the CLI is terminated via a signal, the child process monero-wallet-rpc is not automatically terminated. This discrepancy arises from the behavior of terminal-initiated interrupts (e.g., STRG-C), which send the termination signal to all processes in the process group, effectively killing the child process. To address this, we implement a signal handler that explicitly terminates the monero-wallet-rpc child process upon receiving a termination signal for the parent process. This ensures that the child process does not continue running in the background, detached from the parent. Failing to terminate the child process would lock the wallet, preventing future instances of the CLI from starting.
binarybaron
force-pushed
the
kill-monero-rpc-on-signal-receive
branch
from
78049a9
to
96af3fd
Compare
|
@delta1 do you know why the windows binaries arent getting attached as artifacts? I only have a windows VM and struggling to compile on my machine. |
|
oh right, it’s missing the exe extensions on the upload artifact step. i had a fix for this somewhere, let me try find it or redo it. |
|
Ok, this does not work on windows due to the tokio version upgrade... Swap terminates with |
|
Also doesn't work on tokio 1.21.0. @delta1 Do you have an idea what could be causing this? I'm currently looking through the tokio changelog to see if I find anything https://github.com/tokio-rs/tokio/blob/master/tokio/CHANGELOG.md |
does this happen immediately or only when it catches the signal? |
|
Immediately. It's definitely due to the Tokio upgrade. Upgrading Tokio on master without the other changes also caused a crash. It also crashes on commands like Maybe we really need to increase the stack size but I wonder why. |
Update: This is not related to the tokio upgrade. All debug releases on windows currently panic due to a stackoverflow which means that this PR does not break anything. |
|
Closing this for now... |
|
@binarybaron did you ever find if this could be merged without breaking windows builds. Makes spawning the swap tool programmatically quite annoying to clean up. |
|
No I haven't but feel free to open a PR. I'll happily review and merge it if it works on all platforms (or at least doesn't break any platforms) |
…r SIGHUP
When the CLI is terminated via a signal, the child process monero-wallet-rpc is not automatically terminated. This discrepancy arises from the behavior of terminal-initiated interrupts (e.g., STRG-C), which send the termination signal to all processes in the process group, effectively killing the child process.
To address this, we implement a signal handler that explicitly terminates the monero-wallet-rpc child process upon receiving a termination signal for the parent process. This ensures that the child process does not continue running in the background, detached from the parent. Failing to terminate the child process would lock the wallet, preventing future instances of the CLI from starting.
Fixes #835