From 57844da0e90b00a79359db051eff4a75ac19c05f Mon Sep 17 00:00:00 2001 From: shamardy Date: Wed, 12 Mar 2025 18:24:47 +0200 Subject: [PATCH 01/13] align mm2_main error handling between native and WASM implementations --- mm2src/coins/lp_coins.rs | 4 +- mm2src/mm2_bin_lib/src/lib.rs | 17 +++ mm2src/mm2_bin_lib/src/mm2_native_lib.rs | 83 ++++++++----- mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs | 109 +++++++++++------- mm2src/mm2_main/src/lp_native_dex.rs | 13 +-- mm2src/mm2_main/src/mm2.rs | 27 ++++- .../tests/integration_tests_common/mod.rs | 5 +- 7 files changed, 167 insertions(+), 91 deletions(-) diff --git a/mm2src/coins/lp_coins.rs b/mm2src/coins/lp_coins.rs index 1888e81ad4..8d0bc1cc9c 100644 --- a/mm2src/coins/lp_coins.rs +++ b/mm2src/coins/lp_coins.rs @@ -4997,7 +4997,7 @@ pub async fn delegations_info(ctx: MmArc, req: DelegationsInfo) -> Result { let MmCoinEnum::QtumCoin(qtum) = coin else { return MmError::err(StakingInfoError::InvalidPayload { - reason: format!("{} is not a Qtum coin", req.coin) + reason: format!("{} is not a Qtum coin", req.coin), }); }; @@ -5048,7 +5048,7 @@ pub async fn claim_staking_rewards(ctx: MmArc, req: ClaimStakingRewardsRequest) let MmCoinEnum::Tendermint(tendermint) = coin else { return MmError::err(DelegationError::InvalidPayload { - reason: format!("{} is not a Cosmos coin", req.coin) + reason: format!("{} is not a Cosmos coin", req.coin), }); }; diff --git a/mm2src/mm2_bin_lib/src/lib.rs b/mm2src/mm2_bin_lib/src/lib.rs index 713121b373..0b9db9f011 100644 --- a/mm2src/mm2_bin_lib/src/lib.rs +++ b/mm2src/mm2_bin_lib/src/lib.rs @@ -108,3 +108,20 @@ async fn finalize_mm2_stop(ctx: MmArc) { dispatch_lp_event(ctx.clone(), StopCtxEvent.into()).await; let _ = ctx.stop().await; } + +#[cfg_attr(target_arch = "wasm32", derive(serde::Serialize))] +#[derive(Clone, Debug, PartialEq, Primitive)] +pub enum StartupErrorCode { + /// Operation completed successfully + Ok = 0, + /// Invalid parameters were provided to the function + InvalidParams = 1, + /// The configuration was invalid (missing required fields, etc.) + ConfigError = 2, + /// MM2 is already running + AlreadyRunning = 3, + /// MM2 initialization failed + InitError = 4, + /// Failed to spawn the MM2 process/thread + SpawnError = 5, +} diff --git a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs index f08a0f756b..ad2792fb22 100644 --- a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs @@ -7,6 +7,7 @@ use enum_primitive_derive::Primitive; use gstuff::any_to_str; use libc::c_char; use mm2_core::mm_ctx::MmArc; +use mm2_main::LpMainParams; use num_traits::FromPrimitive; use serde_json::{self as json}; use std::ffi::{CStr, CString}; @@ -15,15 +16,6 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::thread; use std::time::Duration; -#[derive(Debug, PartialEq, Primitive)] -enum MainErr { - Ok = 0, - AlreadyRuns = 1, - ConfIsNull = 2, - ConfNotUtf8 = 3, - CantThread = 5, -} - /// Starts the MM2 in a detached singleton thread. #[no_mangle] #[allow(clippy::missing_safety_doc)] @@ -48,40 +40,75 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin }}; } - if LP_MAIN_RUNNING.load(Ordering::Relaxed) { - eret!(MainErr::AlreadyRuns) - } - CTX.store(0, Ordering::Relaxed); // Remove the old context ID during restarts. - if conf.is_null() { - eret!(MainErr::ConfIsNull) + eret!(StartupErrorCode::InvalidParams, "Configuration is null") } - let conf = CStr::from_ptr(conf); - let conf = match conf.to_str() { + let conf_cstr = CStr::from_ptr(conf); + let conf_str = match conf_cstr.to_str() { Ok(s) => s, - Err(e) => eret!(MainErr::ConfNotUtf8, e), + Err(e) => eret!( + StartupErrorCode::InvalidParams, + format!("Configuration is not valid UTF-8: {}", e) + ), + }; + + let conf: json::Value = match json::from_str(conf_str) { + Ok(v) => v, + Err(e) => eret!( + StartupErrorCode::ConfigError, + format!("Failed to parse configuration: {}", e) + ), }; - let conf = conf.to_owned(); + + if LP_MAIN_RUNNING.load(Ordering::Relaxed) { + eret!(StartupErrorCode::AlreadyRunning, "MM2 is already running"); + } + + CTX.store(0, Ordering::Relaxed); // Remove the old context ID during restarts. register_callback(FfiCallback::with_ffi_function(log_cb)); + let rc = thread::Builder::new().name("lp_main".into()).spawn(move || { if let Err(true) = LP_MAIN_RUNNING.compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) { log!("lp_main already started!"); return; } + let ctx_cb = &|ctx| CTX.store(ctx, Ordering::Relaxed); - match catch_unwind(move || mm2_main::run_lp_main(Some(&conf), ctx_cb, KDF_VERSION.into(), KDF_DATETIME.into())) - { - Ok(Ok(_)) => log!("run_lp_main finished"), - Ok(Err(err)) => log!("run_lp_main error: {}", err), - Err(err) => log!("run_lp_main panic: {:?}", any_to_str(&*err)), + + match catch_unwind(move || { + let params = LpMainParams::with_conf(conf).log_filter(None); + + match block_on(mm2_main::lp_main( + params, + &ctx_cb, + KDF_VERSION.into(), + KDF_DATETIME.into(), + )) { + Ok(ctx) => { + if let Err(e) = block_on(mm2_main::lp_run(ctx)) { + log!("MM2 runtime error: {}", e); + } + }, + Err(e) => log!("MM2 initialization failed: {}", e), + } + }) { + Ok(_) => log!("MM2 thread completed normally"), + Err(err) => log!("MM2 thread panicked: {:?}", any_to_str(&*err)), }; + LP_MAIN_RUNNING.store(false, Ordering::Relaxed) }); + if let Err(e) = rc { - eret!(MainErr::CantThread, e) + LP_MAIN_RUNNING.store(false, Ordering::Relaxed); + eret!( + StartupErrorCode::SpawnError, + format!("Failed to spawn MM2 thread: {:?}", e) + ); } - MainErr::Ok as i8 + + StartupErrorCode::Ok as i8 } /// Checks if the MM2 singleton thread is currently running or not. @@ -177,8 +204,8 @@ pub extern "C" fn mm2_test(torch: i32, log_cb: extern "C" fn(line: *const c_char log!("mm2_test] Restarting MM…"); let conf = CString::new(&conf[..]).unwrap(); let rc = unsafe { mm2_main(conf.as_ptr(), log_cb) }; - let rc = MainErr::from_i8(rc).unwrap(); - if rc != MainErr::Ok { + let rc = StartupErrorCode::from_i8(rc).unwrap(); + if rc != StartupErrorCode::Ok { log!("!mm2_main: {:?}", rc); return -1; } diff --git a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs index deaec8fedb..87cef7202a 100644 --- a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs @@ -1,8 +1,5 @@ //! Some specifics of using the [`wasm_bindgen`] library: //! -//! # Currently only `Result` is allowed -//! [tracking issue]: https://github.com/rustwasm/wasm-bindgen/issues/1004 -//! //! # JavaScript enums do not support methods at all //! [tracking issue]: https://github.com/rustwasm/wasm-bindgen/issues/1715 //! @@ -21,18 +18,27 @@ use mm2_rpc::data::legacy::MmVersionResponse; use mm2_rpc::wasm_rpc::WasmRpcResponse; use serde::{Deserialize, Serialize}; use serde_json::Value as Json; - -/// The errors can be thrown when using the `mm2_main` function incorrectly. #[wasm_bindgen] -#[derive(Primitive)] -pub enum Mm2MainErr { - AlreadyRuns = 1, - InvalidParams = 2, - NoCoinsInConf = 3, +#[derive(Debug, Clone, Serialize)] +struct StartupError { + code: StartupErrorCode, + message: String, } -impl From for JsValue { - fn from(e: Mm2MainErr) -> Self { JsValue::from(e as i32) } +#[wasm_bindgen] +impl StartupError { + fn new(code: StartupErrorCode, message: impl Into) -> Self { + Self { + code, + message: message.into(), + } + } + + #[wasm_bindgen(getter)] + pub fn code(&self) -> i8 { self.code.clone() as i8 } + + #[wasm_bindgen(getter)] + pub fn message(&self) -> String { self.message.clone() } } #[derive(Deserialize)] @@ -58,7 +64,7 @@ impl From for LpMainParams { /// # Usage /// /// ```javascript -/// import init, {mm2_main, LogLevel, Mm2MainErr} from "./path/to/mm2.js"; +/// import init, {mm2_main, LogLevel, StartupErrorCode} from "./path/to/mm2.js"; /// /// const params = { /// conf: { "gui":"WASMTEST", mm2:1, "passphrase":"YOUR_PASSPHRASE_HERE", "rpc_password":"test123", "coins":[{"coin":"ETH","protocol":{"type":"ETH"}}] }, @@ -68,8 +74,8 @@ impl From for LpMainParams { /// try { /// mm2_main(params, handle_log); /// } catch (e) { -/// switch (e) { -/// case Mm2MainErr.AlreadyRuns: +/// switch (e.code) { +/// case StartupErrorCode.AlreadyRunning: /// alert("MarketMaker2 already runs..."); /// break; /// // handle other errors... @@ -80,51 +86,70 @@ impl From for LpMainParams { /// } /// ``` #[wasm_bindgen] -pub fn mm2_main(params: JsValue, log_cb: js_sys::Function) -> Result<(), JsValue> { +pub async fn mm2_main(params: JsValue, log_cb: js_sys::Function) -> Result { let params: MainParams = match deserialize_from_js(params.clone()) { Ok(p) => p, Err(e) => { - console_err!("Expected 'MainParams' as the first argument, found {:?}: {}", params, e); - return Err(Mm2MainErr::InvalidParams.into()); + let error = StartupError::new( + StartupErrorCode::InvalidParams, + format!("Expected 'MainParams' as the first argument, found {:?}: {}", params, e), + ); + console_err!("{}", error.message()); + return Err(error.into()); }, }; if params.conf["coins"].is_null() { - console_err!("Config must contain 'coins' field: {:?}", params.conf); - return Err(Mm2MainErr::NoCoinsInConf.into()); + let error = StartupError::new( + StartupErrorCode::ConfigError, + format!("Config must contain 'coins' field: {:?}", params.conf), + ); + console_err!("{}", error.message()); + return Err(error.into()); } let params = LpMainParams::from(params); if LP_MAIN_RUNNING.load(Ordering::Relaxed) { - return Err(Mm2MainErr::AlreadyRuns.into()); + let error = StartupError::new(StartupErrorCode::AlreadyRunning, "MM2 is already running"); + console_err!("{}", error.message()); + return Err(error.into()); } CTX.store(0, Ordering::Relaxed); // Remove the old context ID during restarts. register_callback(WasmCallback::with_js_function(log_cb)); + // Setting up a global panic hook to log panic information to the console + // This doesn't prevent termination of the WebAssembly instance, but ensures error details are visible + // We can't use catch_unwind directly with async/await in WebAssembly, so this is our best option for diagnostics + // If a panic occurs, the MM2 instance will terminate but the browser tab will remain responsive set_panic_hook(); - let fut = async move { - if let Err(true) = LP_MAIN_RUNNING.compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) { - console_err!("lp_main already started!"); - return; - } - let ctx_cb = |ctx| CTX.store(ctx, Ordering::Relaxed); - // TODO figure out how to use catch_unwind here - // use futures::FutureExt; - // match mm2_main::lp_main(params, &ctx_cb).catch_unwind().await { - // Ok(Ok(_)) => console_info!("run_lp_main finished"), - // Ok(Err(err)) => console_err!("run_lp_main error: {}", err), - // Err(err) => console_err!("run_lp_main panic: {:?}", any_to_str(&*err)), - // }; - match mm2_main::lp_main(params, &ctx_cb, KDF_VERSION.into(), KDF_DATETIME.into()).await { - Ok(()) => console_info!("run_lp_main finished"), - Err(err) => console_err!("run_lp_main error: {}", err), - }; - LP_MAIN_RUNNING.store(false, Ordering::Relaxed) + if let Err(true) = LP_MAIN_RUNNING.compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) { + let error = StartupError::new(StartupErrorCode::AlreadyRunning, "lp_main already started!"); + console_err!("{}", error.message()); + return Err(error.into()); + } + + let ctx_cb = |ctx| CTX.store(ctx, Ordering::Relaxed); + let ctx = match mm2_main::lp_main(params, &ctx_cb, KDF_VERSION.into(), KDF_DATETIME.into()).await { + Ok(ctx) => { + console_info!("run_lp_main finished"); + ctx + }, + Err(err) => { + let error = StartupError::new(StartupErrorCode::InitError, format!("run_lp_main error: {}", err)); + console_err!("{}", error.message()); + LP_MAIN_RUNNING.store(false, Ordering::Relaxed); + return Err(error.into()); + }, }; - // At this moment we still don't have `MmCtx` context to use its `MmCtx::abortable_system` spawner. - executor::spawn_local(fut); - Ok(()) + executor::spawn_local(async move { + if let Err(e) = mm2_main::lp_run(ctx).await { + console_err!("MM2 runtime error: {}", e); + } + LP_MAIN_RUNNING.store(false, Ordering::Relaxed); + }); + + Ok(StartupErrorCode::Ok as i8) } /// Returns the MarketMaker2 instance status. diff --git a/mm2src/mm2_main/src/lp_native_dex.rs b/mm2src/mm2_main/src/lp_native_dex.rs index 878a74ea6b..fdad81e9ff 100644 --- a/mm2src/mm2_main/src/lp_native_dex.rs +++ b/mm2src/mm2_main/src/lp_native_dex.rs @@ -20,7 +20,7 @@ use bitcrypto::sha256; use coins::register_balance_update_handler; -use common::executor::{SpawnFuture, Timer}; +use common::executor::SpawnFuture; use common::log::{info, warn}; use crypto::{from_hw_error, CryptoCtx, HwError, HwProcessingError, HwRpcError, WithHwRpcError}; use derive_more::Display; @@ -49,7 +49,6 @@ use crate::lp_message_service::{init_message_service, InitMessageServiceError}; use crate::lp_network::{lp_network_ports, p2p_event_process_loop, subscribe_to_topic, NetIdError}; use crate::lp_ordermatch::{broadcast_maker_orders_keep_alive_loop, clean_memory_loop, init_ordermatch_context, lp_ordermatch_loop, orders_kick_start, BalanceUpdateOrdermatchHandler, OrdermatchInitError}; -use crate::lp_swap; use crate::lp_swap::swap_kick_starts; use crate::lp_wallet::{initialize_wallet_passphrase, WalletInitError}; use crate::rpc::spawn_rpc; @@ -508,16 +507,6 @@ pub async fn lp_init(ctx: MmArc, version: String, datetime: String) -> MmInitRes } }); - // In the mobile version we might depend on `lp_init` staying around until the context stops. - loop { - if ctx.is_stopping() { - break; - }; - Timer::sleep(0.2).await - } - // Clearing up the running swaps removes any circular references that might prevent the context from being dropped. - lp_swap::clear_running_swaps(&ctx); - Ok(()) } diff --git a/mm2src/mm2_main/src/mm2.rs b/mm2src/mm2_main/src/mm2.rs index 65a10e3ea6..5558f62946 100644 --- a/mm2src/mm2_main/src/mm2.rs +++ b/mm2src/mm2_main/src/mm2.rs @@ -42,10 +42,11 @@ #[cfg(not(target_arch = "wasm32"))] use common::block_on; use common::crash_reports::init_crash_reports; +use common::executor::Timer; use common::log; use common::log::LogLevel; use common::password_policy::password_policy; -use mm2_core::mm_ctx::MmCtxBuilder; +use mm2_core::mm_ctx::{MmArc, MmCtxBuilder}; #[cfg(any(feature = "custom-swap-locktime", test, feature = "run-docker-tests"))] use common::log::warn; @@ -126,7 +127,7 @@ pub async fn lp_main( ctx_cb: &dyn Fn(u32), version: String, datetime: String, -) -> Result<(), String> { +) -> Result { let log_filter = params.filter.unwrap_or_default(); // Logger can be initialized once. // If `kdf` is linked as a library, and `kdf` is restarted, `init_logger` returns an error. @@ -166,7 +167,22 @@ pub async fn lp_main( #[cfg(not(target_arch = "wasm32"))] spawn_ctrl_c_handler(ctx.clone()); - try_s!(lp_init(ctx, version, datetime).await); + try_s!(lp_init(ctx.clone(), version, datetime).await); + Ok(ctx) +} + +pub async fn lp_run(ctx: MmArc) -> Result<(), String> { + // In the mobile version we might depend on `lp_init` staying around until the context stops. + loop { + if ctx.is_stopping() { + break; + }; + Timer::sleep(0.2).await + } + + // Clearing up the running swaps removes any circular references that might prevent the context from being dropped. + lp_swap::clear_running_swaps(&ctx); + Ok(()) } @@ -174,7 +190,7 @@ pub async fn lp_main( /// /// 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) { +fn spawn_ctrl_c_handler(ctx: MmArc) { use crate::lp_dispatcher::{dispatch_lp_event, StopCtxEvent}; common::executor::spawn(async move { @@ -369,7 +385,8 @@ pub fn run_lp_main( let log_filter = LogLevel::from_env(); let params = LpMainParams::with_conf(conf).log_filter(log_filter); - try_s!(block_on(lp_main(params, ctx_cb, version, datetime))); + let ctx = try_s!(block_on(lp_main(params, ctx_cb, version, datetime))); + try_s!(block_on(lp_run(ctx))); Ok(()) } diff --git a/mm2src/mm2_main/tests/integration_tests_common/mod.rs b/mm2src/mm2_main/tests/integration_tests_common/mod.rs index 0c22f3c8bd..23d0523d31 100644 --- a/mm2src/mm2_main/tests/integration_tests_common/mod.rs +++ b/mm2src/mm2_main/tests/integration_tests_common/mod.rs @@ -2,7 +2,7 @@ use common::executor::Timer; use common::log::LogLevel; use common::{block_on, log, now_ms, wait_until_ms}; use crypto::privkey::key_pair_from_seed; -use mm2_main::{lp_main, LpMainParams}; +use mm2_main::{lp_main, lp_run, LpMainParams}; use mm2_rpc::data::legacy::CoinInitResponse; use mm2_test_helpers::electrums::{doc_electrums, marty_electrums}; use mm2_test_helpers::for_tests::{create_new_account_status, enable_native as enable_native_impl, @@ -26,7 +26,8 @@ pub fn test_mm_start_impl() { log!("test_mm_start] Starting the MarketMaker..."); let conf: Json = json::from_str(&conf).unwrap(); let params = LpMainParams::with_conf(conf).log_filter(Some(filter)); - block_on(lp_main(params, &|_ctx| (), "TEST".into(), "TEST".into())).unwrap() + let ctx = block_on(lp_main(params, &|_ctx| (), "TEST".into(), "TEST".into())).unwrap(); + block_on(lp_run(ctx)).unwrap() } } } From 6dd335ef7be8bfa3cf6fb2c6ebf441d52af216f9 Mon Sep 17 00:00:00 2001 From: shamardy Date: Wed, 12 Mar 2025 19:05:02 +0200 Subject: [PATCH 02/13] fix test_mm2_stops_immediately --- mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs | 2 +- mm2src/mm2_main/src/wasm_tests.rs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs index 87cef7202a..7dbc2cc5e1 100644 --- a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs @@ -131,7 +131,7 @@ pub async fn mm2_main(params: JsValue, log_cb: js_sys::Function) -> Result { - console_info!("run_lp_main finished"); + console_info!("lp_main finished"); ctx }, Err(err) => { diff --git a/mm2src/mm2_main/src/wasm_tests.rs b/mm2src/mm2_main/src/wasm_tests.rs index 6aace5bfbc..ca77e907e0 100644 --- a/mm2src/mm2_main/src/wasm_tests.rs +++ b/mm2src/mm2_main/src/wasm_tests.rs @@ -1,4 +1,4 @@ -use crate::lp_init; +use crate::{lp_init, lp_run}; use common::executor::{spawn, spawn_abortable, spawn_local_abortable, AbortOnDropHandle, Timer}; use common::log::warn; use common::log::wasm_log::register_wasm_log; @@ -22,7 +22,8 @@ const STOP_TIMEOUT_MS: u64 = 1000; /// Starts the WASM version of MM. fn wasm_start(ctx: MmArc) { spawn(async move { - lp_init(ctx, "TEST".into(), "TEST".into()).await.unwrap(); + lp_init(ctx.clone(), "TEST".into(), "TEST".into()).await.unwrap(); + lp_run(ctx).await.unwrap(); }) } From 64aa8997730cf0a192199ade05c05c189be90457 Mon Sep 17 00:00:00 2001 From: shamardy Date: Wed, 12 Mar 2025 19:07:34 +0200 Subject: [PATCH 03/13] fix ci issue --- .github/workflows/fmt-and-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/fmt-and-lint.yml b/.github/workflows/fmt-and-lint.yml index b5212320dd..27e7f0cb85 100644 --- a/.github/workflows/fmt-and-lint.yml +++ b/.github/workflows/fmt-and-lint.yml @@ -18,7 +18,7 @@ jobs: - name: Install toolchain run: | - rustup toolchain install nightly-2023-06-01 --no-self-update --profile=minimal --component rustfmt clippy + rustup toolchain install nightly-2023-06-01 --no-self-update --profile=minimal --component rustfmt,clippy rustup default nightly-2023-06-01 - name: Install build deps From c26088dc1126e6f0aa8b0300c2943fbe395a5c0d Mon Sep 17 00:00:00 2001 From: shamardy Date: Wed, 19 Mar 2025 00:57:45 +0200 Subject: [PATCH 04/13] review fix: rename `StartupErrorCode` to `StartupResultCode` --- mm2src/mm2_bin_lib/src/lib.rs | 2 +- mm2src/mm2_bin_lib/src/mm2_native_lib.rs | 6 +++--- mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs | 16 ++++++++-------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/mm2src/mm2_bin_lib/src/lib.rs b/mm2src/mm2_bin_lib/src/lib.rs index 0b9db9f011..dd0c4397b6 100644 --- a/mm2src/mm2_bin_lib/src/lib.rs +++ b/mm2src/mm2_bin_lib/src/lib.rs @@ -111,7 +111,7 @@ async fn finalize_mm2_stop(ctx: MmArc) { #[cfg_attr(target_arch = "wasm32", derive(serde::Serialize))] #[derive(Clone, Debug, PartialEq, Primitive)] -pub enum StartupErrorCode { +pub enum StartupResultCode { /// Operation completed successfully Ok = 0, /// Invalid parameters were provided to the function diff --git a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs index ad2792fb22..58e0f8862c 100644 --- a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs @@ -108,7 +108,7 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin ); } - StartupErrorCode::Ok as i8 + StartupResultCode::Ok as i8 } /// Checks if the MM2 singleton thread is currently running or not. @@ -204,8 +204,8 @@ pub extern "C" fn mm2_test(torch: i32, log_cb: extern "C" fn(line: *const c_char log!("mm2_test] Restarting MM…"); let conf = CString::new(&conf[..]).unwrap(); let rc = unsafe { mm2_main(conf.as_ptr(), log_cb) }; - let rc = StartupErrorCode::from_i8(rc).unwrap(); - if rc != StartupErrorCode::Ok { + let rc = StartupResultCode::from_i8(rc).unwrap(); + if rc != StartupResultCode::Ok { log!("!mm2_main: {:?}", rc); return -1; } diff --git a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs index 7dbc2cc5e1..7a52045aaf 100644 --- a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs @@ -21,13 +21,13 @@ use serde_json::Value as Json; #[wasm_bindgen] #[derive(Debug, Clone, Serialize)] struct StartupError { - code: StartupErrorCode, + code: StartupResultCode, message: String, } #[wasm_bindgen] impl StartupError { - fn new(code: StartupErrorCode, message: impl Into) -> Self { + fn new(code: StartupResultCode, message: impl Into) -> Self { Self { code, message: message.into(), @@ -91,7 +91,7 @@ pub async fn mm2_main(params: JsValue, log_cb: js_sys::Function) -> Result p, Err(e) => { let error = StartupError::new( - StartupErrorCode::InvalidParams, + StartupResultCode::InvalidParams, format!("Expected 'MainParams' as the first argument, found {:?}: {}", params, e), ); console_err!("{}", error.message()); @@ -100,7 +100,7 @@ pub async fn mm2_main(params: JsValue, log_cb: js_sys::Function) -> Result Result Result Result { - let error = StartupError::new(StartupErrorCode::InitError, format!("run_lp_main error: {}", err)); + let error = StartupError::new(StartupResultCode::InitError, format!("run_lp_main error: {}", err)); console_err!("{}", error.message()); LP_MAIN_RUNNING.store(false, Ordering::Relaxed); return Err(error.into()); @@ -149,7 +149,7 @@ pub async fn mm2_main(params: JsValue, log_cb: js_sys::Function) -> Result Date: Wed, 19 Mar 2025 01:46:31 +0200 Subject: [PATCH 05/13] review fix: use `StartupResultCode` in `mm2_test` where appropriate --- mm2src/mm2_bin_lib/src/mm2_native_lib.rs | 20 ++++++++++---------- mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs index 58e0f8862c..aba803419a 100644 --- a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs @@ -41,13 +41,13 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin } if conf.is_null() { - eret!(StartupErrorCode::InvalidParams, "Configuration is null") + eret!(StartupResultCode::InvalidParams, "Configuration is null") } let conf_cstr = CStr::from_ptr(conf); let conf_str = match conf_cstr.to_str() { Ok(s) => s, Err(e) => eret!( - StartupErrorCode::InvalidParams, + StartupResultCode::InvalidParams, format!("Configuration is not valid UTF-8: {}", e) ), }; @@ -55,13 +55,13 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin let conf: json::Value = match json::from_str(conf_str) { Ok(v) => v, Err(e) => eret!( - StartupErrorCode::ConfigError, + StartupResultCode::ConfigError, format!("Failed to parse configuration: {}", e) ), }; if LP_MAIN_RUNNING.load(Ordering::Relaxed) { - eret!(StartupErrorCode::AlreadyRunning, "MM2 is already running"); + eret!(StartupResultCode::AlreadyRunning, "MM2 is already running"); } CTX.store(0, Ordering::Relaxed); // Remove the old context ID during restarts. @@ -103,7 +103,7 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin if let Err(e) = rc { LP_MAIN_RUNNING.store(false, Ordering::Relaxed); eret!( - StartupErrorCode::SpawnError, + StartupResultCode::SpawnError, format!("Failed to spawn MM2 thread: {:?}", e) ); } @@ -136,7 +136,7 @@ pub extern "C" fn mm2_test(torch: i32, log_cb: extern "C" fn(line: *const c_char static RUNNING: AtomicBool = AtomicBool::new(false); if let Err(true) = RUNNING.compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) { log!("mm2_test] Running already!"); - return -1; + return StartupResultCode::AlreadyRunning as i32; } // #402: Stop the MM in order to test the library restart. @@ -147,7 +147,7 @@ pub extern "C" fn mm2_test(torch: i32, log_cb: extern "C" fn(line: *const c_char Ok(ctx) => ctx, Err(err) => { log!("mm2_test] Invalid CTX? !from_ffi_handle: {}", err); - return -1; + return StartupResultCode::InvalidParams as i32; }, }; let conf = json::to_string(&ctx.conf).unwrap(); @@ -207,7 +207,7 @@ pub extern "C" fn mm2_test(torch: i32, log_cb: extern "C" fn(line: *const c_char let rc = StartupResultCode::from_i8(rc).unwrap(); if rc != StartupResultCode::Ok { log!("!mm2_main: {:?}", rc); - return -1; + return rc as i32; } // Wait for the new MM instance to allocate context. @@ -219,14 +219,14 @@ pub extern "C" fn mm2_test(torch: i32, log_cb: extern "C" fn(line: *const c_char } if now_float() - since > 60.0 { log!("mm2_test] Won't start"); - return -1; + return StartupResultCode::InitError as i32; } } let ctx_id = CTX.load(Ordering::Relaxed); if ctx_id == prev_ctx_id { log!("mm2_test] Context ID is the same"); - return -1; + return StartupResultCode::InvalidParams as i32; } log!("mm2_test] New MM instance {} started", ctx_id); } diff --git a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs index 7a52045aaf..09a2cd933d 100644 --- a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs @@ -64,7 +64,7 @@ impl From for LpMainParams { /// # Usage /// /// ```javascript -/// import init, {mm2_main, LogLevel, StartupErrorCode} from "./path/to/mm2.js"; +/// import init, {mm2_main, LogLevel, StartupResultCode} from "./path/to/mm2.js"; /// /// const params = { /// conf: { "gui":"WASMTEST", mm2:1, "passphrase":"YOUR_PASSPHRASE_HERE", "rpc_password":"test123", "coins":[{"coin":"ETH","protocol":{"type":"ETH"}}] }, @@ -75,7 +75,7 @@ impl From for LpMainParams { /// mm2_main(params, handle_log); /// } catch (e) { /// switch (e.code) { -/// case StartupErrorCode.AlreadyRunning: +/// case StartupResultCode.AlreadyRunning: /// alert("MarketMaker2 already runs..."); /// break; /// // handle other errors... From bd480a45f2a6b770b032697de298a400cc2e434a Mon Sep 17 00:00:00 2001 From: shamardy Date: Wed, 19 Mar 2025 01:52:56 +0200 Subject: [PATCH 06/13] review fix: make `StartupResultCode` copyable --- mm2src/mm2_bin_lib/src/lib.rs | 2 +- mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mm2src/mm2_bin_lib/src/lib.rs b/mm2src/mm2_bin_lib/src/lib.rs index dd0c4397b6..453f1b55f5 100644 --- a/mm2src/mm2_bin_lib/src/lib.rs +++ b/mm2src/mm2_bin_lib/src/lib.rs @@ -110,7 +110,7 @@ async fn finalize_mm2_stop(ctx: MmArc) { } #[cfg_attr(target_arch = "wasm32", derive(serde::Serialize))] -#[derive(Clone, Debug, PartialEq, Primitive)] +#[derive(Clone, Copy, Debug, PartialEq, Primitive)] pub enum StartupResultCode { /// Operation completed successfully Ok = 0, diff --git a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs index 09a2cd933d..afd6a2dc41 100644 --- a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs @@ -35,7 +35,7 @@ impl StartupError { } #[wasm_bindgen(getter)] - pub fn code(&self) -> i8 { self.code.clone() as i8 } + pub fn code(&self) -> i8 { self.code as i8 } #[wasm_bindgen(getter)] pub fn message(&self) -> String { self.message.clone() } From 80b9198af3abdfe3b96850f10dfbc513017e481f Mon Sep 17 00:00:00 2001 From: shamardy Date: Wed, 19 Mar 2025 14:50:11 +0200 Subject: [PATCH 07/13] wip --- mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs index afd6a2dc41..bf987c2d52 100644 --- a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs @@ -135,7 +135,7 @@ pub async fn mm2_main(params: JsValue, log_cb: js_sys::Function) -> Result { - let error = StartupError::new(StartupResultCode::InitError, format!("run_lp_main error: {}", err)); + let error = StartupError::new(StartupResultCode::InitError, format!("lp_main error: {}", err)); console_err!("{}", error.message()); LP_MAIN_RUNNING.store(false, Ordering::Relaxed); return Err(error.into()); From 590e861faf6153d38295a2f68546ad73f4421c60 Mon Sep 17 00:00:00 2001 From: shamardy Date: Thu, 10 Apr 2025 18:09:43 +0200 Subject: [PATCH 08/13] review fix: move comment above statement --- mm2src/mm2_bin_lib/src/mm2_native_lib.rs | 3 ++- mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs index aba803419a..d70c049331 100644 --- a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs @@ -64,7 +64,8 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin eret!(StartupResultCode::AlreadyRunning, "MM2 is already running"); } - CTX.store(0, Ordering::Relaxed); // Remove the old context ID during restarts. + // Remove the old context ID during restarts. + CTX.store(0, Ordering::Relaxed); register_callback(FfiCallback::with_ffi_function(log_cb)); diff --git a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs index bf987c2d52..427bbc546d 100644 --- a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs @@ -113,7 +113,9 @@ pub async fn mm2_main(params: JsValue, log_cb: js_sys::Function) -> Result Date: Thu, 10 Apr 2025 20:19:09 +0200 Subject: [PATCH 09/13] review fix: properly propagate initialization errors to main thread - Remove unnecessary Result return type from lp_run - Add channel-based communication for initialization status - Add configurable startup timeout with default of 60 seconds --- mm2src/mm2_bin_lib/src/mm2_native_lib.rs | 59 +++++++++++++++++-- mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs | 4 +- mm2src/mm2_main/src/mm2.rs | 6 +- mm2src/mm2_main/src/wasm_tests.rs | 2 +- .../tests/integration_tests_common/mod.rs | 2 +- 5 files changed, 59 insertions(+), 14 deletions(-) diff --git a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs index d70c049331..99b6aa6289 100644 --- a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs @@ -16,6 +16,9 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::thread; use std::time::Duration; +/// Default timeout in seconds for lp_main to run successfully. +const DEFAULT_STARTUP_TIMEOUT: u64 = 60; + /// Starts the MM2 in a detached singleton thread. #[no_mangle] #[allow(clippy::missing_safety_doc)] @@ -60,6 +63,11 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin ), }; + let startup_timeout = conf + .get("startup_timeout") + .and_then(|v| v.as_u64()) + .unwrap_or(DEFAULT_STARTUP_TIMEOUT); + if LP_MAIN_RUNNING.load(Ordering::Relaxed) { eret!(StartupResultCode::AlreadyRunning, "MM2 is already running"); } @@ -69,14 +77,24 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin register_callback(FfiCallback::with_ffi_function(log_cb)); + let (init_sender, init_receiver) = std::sync::mpsc::channel(); + let rc = thread::Builder::new().name("lp_main".into()).spawn(move || { if let Err(true) = LP_MAIN_RUNNING.compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) { log!("lp_main already started!"); + if let Err(err) = init_sender.send(StartupResultCode::AlreadyRunning) { + log!( + "Warning: Could not send already running status - receiver disconnected error: {}", + err + ); + } return; } let ctx_cb = &|ctx| CTX.store(ctx, Ordering::Relaxed); + let inner_sender = init_sender.clone(); + match catch_unwind(move || { let params = LpMainParams::with_conf(conf).log_filter(None); @@ -87,15 +105,36 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin KDF_DATETIME.into(), )) { Ok(ctx) => { - if let Err(e) = block_on(mm2_main::lp_run(ctx)) { - log!("MM2 runtime error: {}", e); + if let Err(err) = inner_sender.send(StartupResultCode::Ok) { + log!( + "Warning: Could not send initialization status - receiver disconnected error: {}", + err + ); + } + + block_on(mm2_main::lp_run(ctx)); + }, + Err(e) => { + log!("MM2 initialization failed: {}", e); + if let Err(err) = inner_sender.send(StartupResultCode::InitError) { + log!( + "Warning: Could not send initialization error - receiver disconnected error: {}", + err + ); } }, - Err(e) => log!("MM2 initialization failed: {}", e), } }) { Ok(_) => log!("MM2 thread completed normally"), - Err(err) => log!("MM2 thread panicked: {:?}", any_to_str(&*err)), + Err(err) => { + log!("MM2 thread panicked: {:?}", any_to_str(&*err)); + if let Err(err) = init_sender.send(StartupResultCode::InitError) { + log!( + "Warning: Could not send panic error - receiver disconnected error: {}", + err + ); + } + }, }; LP_MAIN_RUNNING.store(false, Ordering::Relaxed) @@ -109,7 +148,17 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin ); } - StartupResultCode::Ok as i8 + match init_receiver.recv_timeout(Duration::from_secs(startup_timeout)) { + Ok(status) => status as i8, + Err(e) => { + log!( + "Timeout waiting for MM2 initialization after {} seconds: {:?}", + startup_timeout, + e + ); + StartupResultCode::InitError as i8 + }, + } } /// Checks if the MM2 singleton thread is currently running or not. diff --git a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs index 427bbc546d..9fcf795012 100644 --- a/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_wasm_lib.rs @@ -145,9 +145,7 @@ pub async fn mm2_main(params: JsValue, log_cb: js_sys::Function) -> Result Result<(), String> { +pub async fn lp_run(ctx: MmArc) { // In the mobile version we might depend on `lp_init` staying around until the context stops. loop { if ctx.is_stopping() { @@ -182,8 +182,6 @@ pub async fn lp_run(ctx: MmArc) -> Result<(), String> { // Clearing up the running swaps removes any circular references that might prevent the context from being dropped. lp_swap::clear_running_swaps(&ctx); - - Ok(()) } /// Handles CTRL-C signals and shutdowns the KDF runtime gracefully. @@ -386,7 +384,7 @@ pub fn run_lp_main( let params = LpMainParams::with_conf(conf).log_filter(log_filter); let ctx = try_s!(block_on(lp_main(params, ctx_cb, version, datetime))); - try_s!(block_on(lp_run(ctx))); + block_on(lp_run(ctx)); Ok(()) } diff --git a/mm2src/mm2_main/src/wasm_tests.rs b/mm2src/mm2_main/src/wasm_tests.rs index ca77e907e0..11d31fb91c 100644 --- a/mm2src/mm2_main/src/wasm_tests.rs +++ b/mm2src/mm2_main/src/wasm_tests.rs @@ -23,7 +23,7 @@ const STOP_TIMEOUT_MS: u64 = 1000; fn wasm_start(ctx: MmArc) { spawn(async move { lp_init(ctx.clone(), "TEST".into(), "TEST".into()).await.unwrap(); - lp_run(ctx).await.unwrap(); + lp_run(ctx).await; }) } diff --git a/mm2src/mm2_main/tests/integration_tests_common/mod.rs b/mm2src/mm2_main/tests/integration_tests_common/mod.rs index 23d0523d31..170915f681 100644 --- a/mm2src/mm2_main/tests/integration_tests_common/mod.rs +++ b/mm2src/mm2_main/tests/integration_tests_common/mod.rs @@ -27,7 +27,7 @@ pub fn test_mm_start_impl() { let conf: Json = json::from_str(&conf).unwrap(); let params = LpMainParams::with_conf(conf).log_filter(Some(filter)); let ctx = block_on(lp_main(params, &|_ctx| (), "TEST".into(), "TEST".into())).unwrap(); - block_on(lp_run(ctx)).unwrap() + block_on(lp_run(ctx)) } } } From 299592b2b94e1a341fb9f92f97159d3f3bd8deb0 Mon Sep 17 00:00:00 2001 From: shamardy Date: Fri, 11 Apr 2025 16:21:26 +0200 Subject: [PATCH 10/13] use recv instead of timeout --- mm2src/mm2_bin_lib/src/mm2_native_lib.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs index 99b6aa6289..672764a831 100644 --- a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs @@ -16,9 +16,6 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::thread; use std::time::Duration; -/// Default timeout in seconds for lp_main to run successfully. -const DEFAULT_STARTUP_TIMEOUT: u64 = 60; - /// Starts the MM2 in a detached singleton thread. #[no_mangle] #[allow(clippy::missing_safety_doc)] @@ -63,11 +60,6 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin ), }; - let startup_timeout = conf - .get("startup_timeout") - .and_then(|v| v.as_u64()) - .unwrap_or(DEFAULT_STARTUP_TIMEOUT); - if LP_MAIN_RUNNING.load(Ordering::Relaxed) { eret!(StartupResultCode::AlreadyRunning, "MM2 is already running"); } @@ -148,14 +140,10 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin ); } - match init_receiver.recv_timeout(Duration::from_secs(startup_timeout)) { + match init_receiver.recv() { Ok(status) => status as i8, Err(e) => { - log!( - "Timeout waiting for MM2 initialization after {} seconds: {:?}", - startup_timeout, - e - ); + log!("Failed to receive initialization status: {}", e); StartupResultCode::InitError as i8 }, } From 77d26f4406881438ac775039577b83944931c74b Mon Sep 17 00:00:00 2001 From: shamardy Date: Fri, 11 Apr 2025 16:36:00 +0200 Subject: [PATCH 11/13] move compare_exchange outside the spawned thread --- mm2src/mm2_bin_lib/src/mm2_native_lib.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs index 672764a831..5998a6ab95 100644 --- a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs @@ -60,7 +60,7 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin ), }; - if LP_MAIN_RUNNING.load(Ordering::Relaxed) { + if let Err(true) = LP_MAIN_RUNNING.compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) { eret!(StartupResultCode::AlreadyRunning, "MM2 is already running"); } @@ -72,17 +72,6 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin let (init_sender, init_receiver) = std::sync::mpsc::channel(); let rc = thread::Builder::new().name("lp_main".into()).spawn(move || { - if let Err(true) = LP_MAIN_RUNNING.compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed) { - log!("lp_main already started!"); - if let Err(err) = init_sender.send(StartupResultCode::AlreadyRunning) { - log!( - "Warning: Could not send already running status - receiver disconnected error: {}", - err - ); - } - return; - } - let ctx_cb = &|ctx| CTX.store(ctx, Ordering::Relaxed); let inner_sender = init_sender.clone(); From 29c82066970e4c9261154197d68acbe9e0d7bf7d Mon Sep 17 00:00:00 2001 From: shamardy Date: Mon, 14 Apr 2025 17:46:40 +0200 Subject: [PATCH 12/13] review fix: run mm2 initialization in main thread --- mm2src/mm2_bin_lib/src/mm2_native_lib.rs | 90 ++++++++++++------------ 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs index 5998a6ab95..c563e6b04d 100644 --- a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs @@ -69,52 +69,58 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin register_callback(FfiCallback::with_ffi_function(log_cb)); - let (init_sender, init_receiver) = std::sync::mpsc::channel(); - - let rc = thread::Builder::new().name("lp_main".into()).spawn(move || { - let ctx_cb = &|ctx| CTX.store(ctx, Ordering::Relaxed); + let ctx_cb = &|ctx| CTX.store(ctx, Ordering::Relaxed); + let params = LpMainParams::with_conf(conf).log_filter(None); + + let ctx = match catch_unwind(|| { + block_on(mm2_main::lp_main( + params, + &ctx_cb, + KDF_VERSION.into(), + KDF_DATETIME.into(), + )) + }) { + Ok(Ok(ctx)) => ctx, + Ok(Err(e)) => { + log!("MM2 initialization failed: {}", e); + LP_MAIN_RUNNING.store(false, Ordering::Relaxed); + return StartupResultCode::InitError as i8; + }, + Err(err) => { + log!("MM2 initialization panicked: {:?}", any_to_str(&*err)); + LP_MAIN_RUNNING.store(false, Ordering::Relaxed); + return StartupResultCode::InitError as i8; + }, + }; - let inner_sender = init_sender.clone(); + // This allows us to use catch_unwind inside the lp_run thread despite MmCtx containing + // types with interior mutability (Mutex, RwLock, etc.) that aren't UnwindSafe. + // By passing just a numeric ID and recovering the actual context inside the + // catch_unwind block, we satisfy the compiler's safety requirements while properly + // handling potential panics. + let ctx_id = match ctx.ffi_handle() { + Ok(id) => id, + Err(e) => { + log!("MM2 thread setup failed: Failed to create FFI handle: {}", e); + LP_MAIN_RUNNING.store(false, Ordering::Relaxed); + return StartupResultCode::InitError as i8; + }, + }; + let rc = thread::Builder::new().name("lp_run".into()).spawn(move || { match catch_unwind(move || { - let params = LpMainParams::with_conf(conf).log_filter(None); - - match block_on(mm2_main::lp_main( - params, - &ctx_cb, - KDF_VERSION.into(), - KDF_DATETIME.into(), - )) { - Ok(ctx) => { - if let Err(err) = inner_sender.send(StartupResultCode::Ok) { - log!( - "Warning: Could not send initialization status - receiver disconnected error: {}", - err - ); - } - - block_on(mm2_main::lp_run(ctx)); + let ctx = match MmArc::from_ffi_handle(ctx_id) { + Ok(ctx) => ctx, + Err(err) => { + log!("Failed to recover context in thread: {:?}", err); + return; }, - Err(e) => { - log!("MM2 initialization failed: {}", e); - if let Err(err) = inner_sender.send(StartupResultCode::InitError) { - log!( - "Warning: Could not send initialization error - receiver disconnected error: {}", - err - ); - } - }, - } + }; + block_on(mm2_main::lp_run(ctx)); }) { Ok(_) => log!("MM2 thread completed normally"), Err(err) => { log!("MM2 thread panicked: {:?}", any_to_str(&*err)); - if let Err(err) = init_sender.send(StartupResultCode::InitError) { - log!( - "Warning: Could not send panic error - receiver disconnected error: {}", - err - ); - } }, }; @@ -129,13 +135,7 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin ); } - match init_receiver.recv() { - Ok(status) => status as i8, - Err(e) => { - log!("Failed to receive initialization status: {}", e); - StartupResultCode::InitError as i8 - }, - } + StartupResultCode::Ok as i8 } /// Checks if the MM2 singleton thread is currently running or not. From 80cfea7360155fe591dcae33d6e0f623e2c46bdc Mon Sep 17 00:00:00 2001 From: shamardy Date: Mon, 14 Apr 2025 23:58:07 +0200 Subject: [PATCH 13/13] review fix: panic if getting ffi handle fails in the thread --- mm2src/mm2_bin_lib/src/mm2_native_lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs index c563e6b04d..8ca51fd4f2 100644 --- a/mm2src/mm2_bin_lib/src/mm2_native_lib.rs +++ b/mm2src/mm2_bin_lib/src/mm2_native_lib.rs @@ -112,8 +112,7 @@ pub unsafe extern "C" fn mm2_main(conf: *const c_char, log_cb: extern "C" fn(lin let ctx = match MmArc::from_ffi_handle(ctx_id) { Ok(ctx) => ctx, Err(err) => { - log!("Failed to recover context in thread: {:?}", err); - return; + panic!("Failed to recover context in thread: {}", err); }, }; block_on(mm2_main::lp_run(ctx));