feat(core): handling CTRL-C signal with graceful shutdown by onur-ozkan · Pull Request #2213 · GLEECBTC/komodo-defi-framework

onur-ozkan · 2024-09-10T10:16:03Z

To Test: cc @KomodoPlatform/qa
For testing on Windows and MacOS. Try CTRL-C on terminal for a running mm2 instance, it should work the same as using stop RPC.

Signed-off-by: onur-ozkan <work@onurozkan.dev>

mariocynicys

LGTM!

mariocynicys · 2024-09-16T09:42:22Z

mm2src/mm2_main/src/rpc/lp_commands/lp_commands_legacy.rs

+        ctx.stop().await.expect("Couldn't stop the KDF runtime.");
    };

    // Please note we shouldn't use `MmCtx::spawner` to spawn this future,


nit: this is not really an issue here since the aborting code is in the last sync code block inside stop() so we won't yield back the execution to the runtime and thus won't die before we should.
We should be fine running the future inside the ctx spawner.

I'm not sure if it's a good idea to spawn a future inside the future pool to clear the pool. Haven't checked that in detail but it may cause the future to deconstruct itself before it finishes. It's better to not touch that part and keep this PR as simple as possible.

Note that, this isn't just about native threads, there is the JS runtime too.

Agreed. It's not much of a difference and safer this way in case someone adds some awaited call after the abortion code.

We might as well then change...

mariocynicys · 2024-09-16T11:22:14Z

mm2src/mm2_main/src/rpc/lp_commands/lp_commands_legacy.rs

+        ctx.stop().await.expect("Couldn't stop the KDF runtime.");
    };

    // Please note we shouldn't use `MmCtx::spawner` to spawn this future,


Agreed. It's not much of a difference and safer this way in case someone adds some awaited call after the abortion code.

We might as well then change...

mariocynicys · 2024-09-16T11:23:56Z

mm2src/mm2_main/src/mm2.rs

+/// It's important to spawn this task as soon as `Ctx` is in the correct state.
+#[cfg(not(target_arch = "wasm32"))]
+fn spawn_ctrl_c_handler(ctx: mm2_core::mm_ctx::MmArc) {
+    ctx.spawner().spawn(async move {


...this to not use the ctx spawner and call stop inside.

Let's use the global spawner here.

Signed-off-by: onur-ozkan <work@onurozkan.dev>

shamardy

Thanks for the PR! I have a few comments.

mm2src/mm2_main/src/mm2.rs

mm2src/mm2_main/src/rpc/lp_commands/lp_commands_legacy.rs

Signed-off-by: onur-ozkan <work@onurozkan.dev>

Cargo.toml

Signed-off-by: onur-ozkan <work@onurozkan.dev>

…trl-c

* dev: fix(orders): fix cancel order race condition using time-based cache (#2232) fix(legacy-swap): taker failed spend maker payment marked as failed (#2199) chore(adex-cli): deprecate adex-cli (#2234) feat(new-RPC): connection healthcheck implementation for peers (#2194) fix(proxy-signature): add message lifetime overflows (#2233) feat(CI): handle remote files in a safer way (#2217) chore(doc): update issue address in README (#2227) fix(merge): remove duplicated db_root function (#2229) feat(wallets): add `get_wallet_names` rpc (#2202) chore(tests): don't use `.wait()` and use `block_on` instead (#2220) fix(native-rpc): remove escaped response body (#2219) fix(clippy): fix coins mod clippy warnings in wasm (#2224) feat(core): handling CTRL-C signal with graceful shutdown (#2213) docs(README): fix typos (#2212) remove the non-sense arguments (#2216) fix(db): stop creating the all-zeroes dir on KDF start (#2218)

* dev: fix(cosmos): fix tx broadcasting error (#2238) chore(solana): remove solana implementation (#2239) chore(cli): remove leftover subcommands from help message (#2235) fix(orders): fix cancel order race condition using time-based cache (#2232) fix(legacy-swap): taker failed spend maker payment marked as failed (#2199) chore(adex-cli): deprecate adex-cli (#2234) feat(new-RPC): connection healthcheck implementation for peers (#2194) fix(proxy-signature): add message lifetime overflows (#2233) feat(CI): handle remote files in a safer way (#2217) chore(doc): update issue address in README (#2227) fix(merge): remove duplicated db_root function (#2229) feat(wallets): add `get_wallet_names` rpc (#2202) chore(tests): don't use `.wait()` and use `block_on` instead (#2220) fix(native-rpc): remove escaped response body (#2219) fix(clippy): fix coins mod clippy warnings in wasm (#2224) feat(core): handling CTRL-C signal with graceful shutdown (#2213) docs(README): fix typos (#2212) remove the non-sense arguments (#2216) fix(db): stop creating the all-zeroes dir on KDF start (#2218)

onur-ozkan added 2 commits September 10, 2024 13:07

extend the shutdown scope of Ctx::stop

7cc5157

Signed-off-by: onur-ozkan <work@onurozkan.dev>

handle ctrl-c signals

07b9a6d

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan added enhancement labels Sep 10, 2024

onur-ozkan changed the title ~~feat(core): handling CTRL-C signals with graceful shutdowns~~ feat(core): handling CTRL-C signal with graceful shutdown Sep 10, 2024

add an important note

ae8d70e

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the graceful-shutdown-on-ctrl-c branch from e7d03ad to ae8d70e Compare September 10, 2024 10:34

This comment was marked as duplicate.

Sign in to view

mariocynicys previously approved these changes Sep 16, 2024

View reviewed changes

mariocynicys reviewed Sep 16, 2024

View reviewed changes

spawn spawn_ctrl_c_handler outside the abortable pool

f86dd5c

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan dismissed mariocynicys’s stale review via f86dd5c September 16, 2024 11:42

mariocynicys previously approved these changes Sep 16, 2024

View reviewed changes

shamardy reviewed Sep 18, 2024

View reviewed changes

mm2src/mm2_main/src/mm2.rs Show resolved Hide resolved

mm2src/mm2_main/src/mm2.rs Show resolved Hide resolved

mm2src/mm2_main/src/rpc/lp_commands/lp_commands_legacy.rs Show resolved Hide resolved

update release profile

7d0f619

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan dismissed mariocynicys’s stale review via 7d0f619 September 18, 2024 08:17

call dispatch_lp_event before the termination

dc26779

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan commented Sep 18, 2024

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

revert LTO

a3791bd

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the graceful-shutdown-on-ctrl-c branch from 4dbf44e to a3791bd Compare September 18, 2024 09:59

shamardy added 2.2.0-beta labels Sep 19, 2024

Merge remote-tracking branch 'origin/dev' into graceful-shutdown-on-c…

a2643f0

…trl-c

shamardy approved these changes Sep 19, 2024

View reviewed changes

shamardy merged commit baa72a7 into dev Sep 19, 2024

shamardy deleted the graceful-shutdown-on-ctrl-c branch September 19, 2024 01:35

shamardy mentioned this pull request Sep 19, 2024

Add Ctrl-C handling support to mm2 #2145

Closed

shamardy mentioned this pull request Nov 19, 2024

chore(release): v2.2.0-beta #2197

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): handling CTRL-C signal with graceful shutdown#2213

feat(core): handling CTRL-C signal with graceful shutdown#2213
shamardy merged 8 commits intodevfrom
graceful-shutdown-on-ctrl-c

onur-ozkan commented Sep 10, 2024 •

edited by shamardy

Loading

Uh oh!

This comment was marked as duplicate.

Uh oh!

mariocynicys left a comment

Uh oh!

mariocynicys Sep 16, 2024

Uh oh!

onur-ozkan Sep 16, 2024

Uh oh!

onur-ozkan Sep 16, 2024

Uh oh!

mariocynicys Sep 16, 2024

Uh oh!

mariocynicys Sep 16, 2024

Uh oh!

mariocynicys Sep 16, 2024

Uh oh!

shamardy left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

onur-ozkan commented Sep 10, 2024 • edited by shamardy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as duplicate.

Uh oh!

mariocynicys left a comment

Choose a reason for hiding this comment

Uh oh!

mariocynicys Sep 16, 2024

Choose a reason for hiding this comment

Uh oh!

onur-ozkan Sep 16, 2024

Choose a reason for hiding this comment

Uh oh!

onur-ozkan Sep 16, 2024

Choose a reason for hiding this comment

Uh oh!

mariocynicys Sep 16, 2024

Choose a reason for hiding this comment

Uh oh!

mariocynicys Sep 16, 2024

Choose a reason for hiding this comment

Uh oh!

mariocynicys Sep 16, 2024

Choose a reason for hiding this comment

Uh oh!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

onur-ozkan commented Sep 10, 2024 •

edited by shamardy

Loading