[r2r] Stop spawned futures #1490

sergeyboyko0791 · 2022-10-03T15:49:55Z

Spawn Transaction History V1 as a future, not in a separate thread.
Add AbortableSystem with the ability to create Future subsystems
Use MmFutSpawner within mm2_libp2p::AtomicDexBehaviour
Spawn kick-started swaps as futures, not in separate threads.

* Add `MmCoin::spawn`

* Add `SpawnSettings`

* Store abort handles by associated `FutureId` not to keep them forever * Remove `wio::Timeout`

* Add `FutureSpawner` trait to `executor`

* Kick-start swaps by spawning them as futures, not as threads * Remove `common::executor::spawn_boxed` * Delete `common/executor.rs` file

* Leave Spawn traits in `spawner.rs` only * Add `AbortableSpawnerWeak` * Rename `CoinFutureSpawner` to `CoinFutSpawner` * Rename `MmSpawner` to `MmFutSpawner`

* `spawn_gossipsub` takes an `SwarmRuntime` instance to spawn futures

* Add `SwapWatcherLock` * Remove `SwapsContext::shutdown_rx`

* Remove `spawn_abortable_with_settings`

* Allow to create an abortable subsystem that will be aborted if the linked supersystem is aborted.

…system

* Run `test_mm2_stops_impl` twice

sergeyboyko0791 · 2022-10-07T15:13:34Z

This PR affects the following parts:

LightningCoin - cc: @shamardy
Swap Watcher - cc: @caglaryucekaya

artemii235

Thanks for the PR! That's the first and very brief review iteration. I think I will have quite a few questions on the next one :)

mm2src/common/executor/native_executor.rs

# Conflicts: # mm2src/coins/eth.rs # mm2src/coins/lightning.rs # mm2src/coins/qrc20.rs # mm2src/coins/solana/spl.rs # mm2src/coins/utxo/slp.rs # mm2src/coins/z_coin.rs # mm2src/mm2_main/src/mm2_tests.rs # mm2src/mm2_test_helpers/src/for_tests.rs

artemii235

Second review, quite brief again.

mm2src/coins/utxo/rpc_clients.rs

mm2src/mm2_core/src/mm_ctx.rs

artemii235

A couple of questions.

mm2src/mm2_net/src/wasm_ws.rs

mm2src/common/executor/abortable_system/abortable_queue.rs

* Add `AbortableQueue` field into `ElectrumClientImpl`, `ElectrumConnection` * Pass a spawner to `ws_transport` * Remove `wasm_ws::ClosingState` by replacing it with `ClosedState`

shamardy

Great Work! Just reviewed the lightning related parts for now, will review the rest of the PR in more details in the next iteration.

mm2src/coins/lightning/ln_events.rs

* Increase `WASM_BINDGEN_TEST_TIMEOUT` to 180 seconds * Avoid unnecessary `spawn` usage

* Avoid using `allow(dead_code)` * Add `spawn_local_abortable` to `wasm_executor` * Avoid unnecessary `spawn_local` usage at `webusb_driver`

sergeyboyko0791 · 2022-10-13T12:53:58Z

@artemii235 @shamardy PR is ready for the next review iteration

artemii235

Next iteration.
@sergeyboyko0791 Also a question: should we maybe call spawner().abort_all() in the disable_coin RPC?

mm2src/mm2_main/src/rpc.rs

mm2src/coins/lightning/ln_utils.rs

sergeyboyko0791 · 2022-10-13T21:20:54Z

This definitely makes sense as some futures may hold a shared pointer to the coin. But I think we first need to fix #1493
For example, if we abort BTC's related futures, Lightning won't work correctly, but its maker/taker orders won't be cancelled.

Also a question: should we maybe call spawner().abort_all() in the disable_coin RPC?

I can leave a comment in this issue

cc: @artemii235

artemii235

Thanks for the great work!

caglaryucekaya

Great work!

caglaryucekaya · 2022-10-14T11:32:43Z

mm2src/mm2_core/src/mm_ctx.rs

+    /// It's used to spawn futures that can be aborted immediately or after a timeout
+    /// on the [`MmArc::stop`] function call.
+    pub abortable_system: AbortableQueue,
+    /// The abortable system is pinned to the `MmCtx` context.


Duplicate comment

shamardy

Thanks for this great enhancement!

shamardy · 2022-10-14T15:30:02Z

mm2src/mm2_core/src/mm_ctx.rs

+        // Notify shutdown listeners.
+        self.graceful_shutdown_registry.abort_all();
+        // Abort spawned futures.
+        self.abortable_system.abort_all();

        #[cfg(not(target_arch = "wasm32"))]
        self.background_processors.lock().unwrap().drain();


I guess background processors can be moved back to LightningCoin as before since they will be dropped now with the coins on calling stop, I added this to the lightning integration checklist #1045 (comment)

sergeyboyko0791 added 23 commits September 26, 2022 19:32

Wrap lightning futures into AbortOnDropHandle

53e3de7

Refactor common::executor by moving to a separate module

43df80b

Add AbortableSpawner that has spawn and spawn_critical

cc04c31

Spawn TX history with CoinFutureSpawner::spawn

132808c

* Add `MmCoin::spawn`

Spawn electrum client with CoinFutureSpawner

414fa1d

* Add `SpawnSettings`

Merge branch 'dev' into stop-spawned-futures

a2b9306

Use CoinFutureSpawner within UtxoArcBuilder

09ffab5

Refactor AbortableSpawner

3511dc2

* Store abort handles by associated `FutureId` not to keep them forever * Remove `wio::Timeout`

Optimize SpawnedFutured by replacing HashMap with Vec

95ec297

Continue integrating MmSpawner

5d74e9b

Refactor ws_transport to avoid unnecessary spawn calls

d0de4ce

Integrate FutureSpawner into RPC task manager

595060b

* Add `FutureSpawner` trait to `executor`

Make spawn and spawn_local unsafe

c2a3b2a

* Kick-start swaps by spawning them as futures, not as threads * Remove `common::executor::spawn_boxed` * Delete `common/executor.rs` file

Make sure AbortableSpawner can't be cloned

7771e79

* Leave Spawn traits in `spawner.rs` only * Add `AbortableSpawnerWeak` * Rename `CoinFutureSpawner` to `CoinFutSpawner` * Rename `MmSpawner` to `MmFutSpawner`

Remove SWARM_RUNTIME from mm2_libp2p

155e972

* `spawn_gossipsub` takes an `SwarmRuntime` instance to spawn futures

Don't use MmArc::on_stop

cf5276f

* Add `SwapWatcherLock` * Remove `SwapsContext::shutdown_rx`

Don't use MmArc::on_stop when spawn_gossipsub

1dabf4f

* Remove `spawn_abortable_with_settings`

Refactor AbortableSpawner to AbortableSystem

4fa1570

* Allow to create an abortable subsystem that will be aborted if the linked supersystem is aborted.

Spawn check_balance_update_loop future via AbortableSimpleMap sub…

17554a1

…system

Ignore unstable tests

2ff04ca

Add test_mm2_stops_immediately WASM test

62b6613

Fix test_mm2_stops_immediately by removing QRC20 coin from test

42f08f9

* Run `test_mm2_stops_impl` twice

Minor changes and fixes

9c06867

sergeyboyko0791 changed the title ~~[wip] Stop spawned futures~~ [r2r] Stop spawned futures Oct 7, 2022

sergeyboyko0791 requested review from artemii235, borngraced, caglaryucekaya and shamardy October 7, 2022 15:12

sergeyboyko0791 self-assigned this Oct 7, 2022

artemii235 requested changes Oct 10, 2022

View reviewed changes

mm2src/common/executor/native_executor.rs Outdated Show resolved Hide resolved

artemii235 requested changes Oct 11, 2022

View reviewed changes

mm2src/coins/utxo/rpc_clients.rs Outdated Show resolved Hide resolved

mm2src/mm2_core/src/mm_ctx.rs Show resolved Hide resolved

artemii235 reviewed Oct 11, 2022

View reviewed changes

mm2src/mm2_net/src/wasm_ws.rs Outdated Show resolved Hide resolved

mm2src/common/executor/abortable_system/abortable_queue.rs Outdated Show resolved Hide resolved

sergeyboyko0791 changed the title ~~[r2r] Stop spawned futures~~ [wip] Stop spawned futures Oct 12, 2022

Refactor ElectrumClient not to use spawn function

4531dc3

* Add `AbortableQueue` field into `ElectrumClientImpl`, `ElectrumConnection` * Pass a spawner to `ws_transport` * Remove `wasm_ws::ClosingState` by replacing it with `ClosedState`

shamardy reviewed Oct 12, 2022

View reviewed changes

mm2src/coins/lightning/ln_events.rs Outdated Show resolved Hide resolved

sergeyboyko0791 added 7 commits October 12, 2022 22:09

Make spawn and spawn_local safe

ce8859d

* Increase `WASM_BINDGEN_TEST_TIMEOUT` to 180 seconds * Avoid unnecessary `spawn` usage

Minor changes

be674da

* Avoid using `allow(dead_code)` * Add `spawn_local_abortable` to `wasm_executor` * Avoid unnecessary `spawn_local` usage at `webusb_driver`

Add GracefulShutdownRegistry and use it for hyper server

186e00a

Fix BCH, SLP tests

a199637

Rename QueueInner::remove_finished to on_finished

522edd9

Remove MmCtx::stop_listeners, MmCtx::on_stop

de59cb6

Spawn save_channel_closing_details with a critical timeout

c904c63

sergeyboyko0791 changed the title ~~[wip] Stop spawned futures~~ [r2r] Stop spawned futures Oct 13, 2022

artemii235 requested changes Oct 13, 2022

View reviewed changes

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

mm2src/coins/lightning/ln_utils.rs Show resolved Hide resolved

artemii235 approved these changes Oct 14, 2022

View reviewed changes

sergeyboyko0791 mentioned this pull request Oct 14, 2022

Deactivate tokens when platform coin is deactivated #1493

Closed

caglaryucekaya approved these changes Oct 14, 2022

View reviewed changes

caglaryucekaya reviewed Oct 14, 2022

View reviewed changes

shamardy approved these changes Oct 14, 2022

View reviewed changes

artemii235 merged commit 4a93790 into dev Oct 17, 2022

artemii235 deleted the stop-spawned-futures branch October 17, 2022 08:05

shamardy mentioned this pull request Nov 1, 2022

[r2r] use task manager to enable lightning #1513

Merged

sergeyboyko0791 mentioned this pull request Nov 17, 2022

[wasm/bug] Handle error when trying to open already opened database #1158

Closed

shamardy mentioned this pull request Feb 17, 2023

[release] 1.0.0-beta #1576

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[r2r] Stop spawned futures #1490

[r2r] Stop spawned futures #1490

sergeyboyko0791 commented Oct 3, 2022 •

edited

Loading

sergeyboyko0791 commented Oct 7, 2022

artemii235 left a comment

artemii235 left a comment

artemii235 left a comment

shamardy left a comment

sergeyboyko0791 commented Oct 13, 2022

artemii235 left a comment

sergeyboyko0791 commented Oct 13, 2022 •

edited

Loading

artemii235 left a comment

caglaryucekaya left a comment

caglaryucekaya Oct 14, 2022

shamardy left a comment

shamardy Oct 14, 2022

[r2r] Stop spawned futures #1490

[r2r] Stop spawned futures #1490

Conversation

sergeyboyko0791 commented Oct 3, 2022 • edited Loading

sergeyboyko0791 commented Oct 7, 2022

artemii235 left a comment

Choose a reason for hiding this comment

artemii235 left a comment

Choose a reason for hiding this comment

artemii235 left a comment

Choose a reason for hiding this comment

shamardy left a comment

Choose a reason for hiding this comment

sergeyboyko0791 commented Oct 13, 2022

artemii235 left a comment

Choose a reason for hiding this comment

sergeyboyko0791 commented Oct 13, 2022 • edited Loading

artemii235 left a comment

Choose a reason for hiding this comment

caglaryucekaya left a comment

Choose a reason for hiding this comment

caglaryucekaya Oct 14, 2022

Choose a reason for hiding this comment

shamardy left a comment

Choose a reason for hiding this comment

shamardy Oct 14, 2022

Choose a reason for hiding this comment

sergeyboyko0791 commented Oct 3, 2022 •

edited

Loading

sergeyboyko0791 commented Oct 13, 2022 •

edited

Loading