From 1bc059e3afc20bfcbe028ee620f3defac3ab9731 Mon Sep 17 00:00:00 2001 From: Alex Koshelev Date: Thu, 12 Feb 2026 16:05:56 -0800 Subject: [PATCH 1/7] Remove unused imports Remove unused `pin` import from `helpers/transport/receive.rs` and `query/runner/reshard_tag.rs`. --- ipa-core/src/helpers/transport/receive.rs | 2 +- ipa-core/src/query/runner/reshard_tag.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ipa-core/src/helpers/transport/receive.rs b/ipa-core/src/helpers/transport/receive.rs index 299433a1c..4e31c1539 100644 --- a/ipa-core/src/helpers/transport/receive.rs +++ b/ipa-core/src/helpers/transport/receive.rs @@ -1,5 +1,5 @@ use std::{ - pin::{Pin, pin}, + pin::Pin, task::{Context, Poll}, }; diff --git a/ipa-core/src/query/runner/reshard_tag.rs b/ipa-core/src/query/runner/reshard_tag.rs index d10c31dac..cb83d67ff 100644 --- a/ipa-core/src/query/runner/reshard_tag.rs +++ b/ipa-core/src/query/runner/reshard_tag.rs @@ -1,5 +1,5 @@ use std::{ - pin::{Pin, pin}, + pin::Pin, task::{Context, Poll}, }; From c3cf6289c31f3e56d06335a7f60bb937eb9e6d25 Mon Sep 17 00:00:00 2001 From: Alex Koshelev Date: Thu, 12 Feb 2026 16:06:01 -0800 Subject: [PATCH 2/7] Remove dead code Gate `OrderedStream` struct and its `Stream` impl behind `#[cfg(all(test, ...))]` since it is only used in tests. Remove unused `metrics::Request` struct from `http_serde.rs` and unused `QueryType`/`Node` enums from `ipa_bench/models.rs`. --- ipa-core/src/bin/helper.rs | 2 +- ipa-core/src/bin/ipa_bench/models.rs | 13 ------------- ipa-core/src/helpers/buffers/ordering_sender.rs | 6 ++++-- ipa-core/src/net/http_serde.rs | 6 ------ ipa-core/src/protocol/boolean/step.rs | 1 + ipa-core/src/test_fixture/sharing.rs | 1 + ipa-step-test/src/lib.rs | 3 +++ 7 files changed, 10 insertions(+), 22 deletions(-) diff --git a/ipa-core/src/bin/helper.rs b/ipa-core/src/bin/helper.rs index e82598dec..9e3f14f46 100644 --- a/ipa-core/src/bin/helper.rs +++ b/ipa-core/src/bin/helper.rs @@ -291,7 +291,7 @@ async fn server(args: ServerArgs, logging_handle: LoggingHandle) -> Result<(), B join(server_handle, shard_server_handle).await; - [query_runtime, http_runtime].map(Runtime::shutdown_background); + let _ = [query_runtime, http_runtime].map(Runtime::shutdown_background); Ok(()) } diff --git a/ipa-core/src/bin/ipa_bench/models.rs b/ipa-core/src/bin/ipa_bench/models.rs index 04256d27c..bb8e9bbfc 100644 --- a/ipa-core/src/bin/ipa_bench/models.rs +++ b/ipa-core/src/bin/ipa_bench/models.rs @@ -133,19 +133,6 @@ pub enum GenericReport { }, } -#[derive(Serialize, Deserialize)] -enum QueryType { - SourceFanout, - TriggerFanout, -} - -#[derive(Serialize, Deserialize)] -enum Node { - Helper1, - Helper2, - Helper3, -} - #[cfg(all(test, unit_test))] mod tests { use super::{Epoch, EventTimestamp}; diff --git a/ipa-core/src/helpers/buffers/ordering_sender.rs b/ipa-core/src/helpers/buffers/ordering_sender.rs index f69288a49..73df74500 100644 --- a/ipa-core/src/helpers/buffers/ordering_sender.rs +++ b/ipa-core/src/helpers/buffers/ordering_sender.rs @@ -9,7 +9,7 @@ use std::{ task::{Context, Poll}, }; -use futures::{Future, Stream, task::Waker}; +use futures::{Future, task::Waker}; use crate::{ helpers::{Message, buffers::circular::CircularBuf}, @@ -493,11 +493,13 @@ impl Future for Close<'_> { /// the next stream that happens to be polled. Ordinarily streams require a /// mutable reference so that they have exclusive access to the underlying state. /// To avoid that happening, don't make more than one stream. +#[cfg(all(test, any(unit_test, feature = "shuttle")))] pub struct OrderedStream> { sender: B, } -impl + Unpin> Stream for OrderedStream { +#[cfg(all(test, any(unit_test, feature = "shuttle")))] +impl + Unpin> futures::Stream for OrderedStream { type Item = Vec; fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { diff --git a/ipa-core/src/net/http_serde.rs b/ipa-core/src/net/http_serde.rs index 2777cf7cb..695de27e0 100644 --- a/ipa-core/src/net/http_serde.rs +++ b/ipa-core/src/net/http_serde.rs @@ -69,12 +69,6 @@ pub mod echo { } pub mod metrics { - - use serde::{Deserialize, Serialize}; - - #[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] - pub struct Request {} - pub const AXUM_PATH: &str = "/metrics"; } diff --git a/ipa-core/src/protocol/boolean/step.rs b/ipa-core/src/protocol/boolean/step.rs index 0128cb037..746de045c 100644 --- a/ipa-core/src/protocol/boolean/step.rs +++ b/ipa-core/src/protocol/boolean/step.rs @@ -19,4 +19,5 @@ pub struct TwoHundredFiftySixBitOpStep(usize); #[cfg(test)] #[derive(CompactStep)] #[step(count = 256, name = "bit")] +#[allow(dead_code)] // used as a type parameter, not constructed directly pub struct DefaultBitStep(usize); diff --git a/ipa-core/src/test_fixture/sharing.rs b/ipa-core/src/test_fixture/sharing.rs index b42ce5c54..29aeb0e5f 100644 --- a/ipa-core/src/test_fixture/sharing.rs +++ b/ipa-core/src/test_fixture/sharing.rs @@ -212,6 +212,7 @@ impl Reconstruct<()> for [(); 3] { fn reconstruct(&self) {} } +#[allow(dead_code)] // used as a trait bound; not all feature combinations call its methods pub trait ValidateMalicious { fn validate(&self, r: F::ExtendedField); } diff --git a/ipa-step-test/src/lib.rs b/ipa-step-test/src/lib.rs index 3789e6fcf..8ea549fd0 100644 --- a/ipa-step-test/src/lib.rs +++ b/ipa-step-test/src/lib.rs @@ -1,5 +1,8 @@ +#[cfg(test)] mod basic_step; +#[cfg(test)] mod complex_step; +#[cfg(test)] mod module; #[cfg(test)] From 95b7ed09605ac86182eaedee55e6365fb8e124f4 Mon Sep 17 00:00:00 2001 From: Alex Koshelev Date: Thu, 12 Feb 2026 16:06:06 -0800 Subject: [PATCH 3/7] Add reasons to `#[ignore]` attributes Provide descriptive reasons for `#[ignore]` tests, satisfying `clippy::ignore_without_reason`. --- .../boolean_ops/comparison_and_subtraction_sequential.rs | 4 ++-- ipa-core/src/protocol/ipa_prf/oprf_padding/mod.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ipa-core/src/protocol/ipa_prf/boolean_ops/comparison_and_subtraction_sequential.rs b/ipa-core/src/protocol/ipa_prf/boolean_ops/comparison_and_subtraction_sequential.rs index 5df9ca0f2..ece7c4c63 100644 --- a/ipa-core/src/protocol/ipa_prf/boolean_ops/comparison_and_subtraction_sequential.rs +++ b/ipa-core/src/protocol/ipa_prf/boolean_ops/comparison_and_subtraction_sequential.rs @@ -375,7 +375,7 @@ mod test { const BENCH_COUNT: usize = 131_072; #[test] - #[ignore] // benchmark + #[ignore = "benchmark"] #[cfg(not(coverage))] fn semi_honest_compare_gt_novec() { run(|| async move { @@ -430,7 +430,7 @@ mod test { } #[test] - #[ignore] // benchmark + #[ignore = "benchmark"] #[cfg(not(coverage))] fn semi_honest_compare_gt_vec() { run(|| async move { diff --git a/ipa-core/src/protocol/ipa_prf/oprf_padding/mod.rs b/ipa-core/src/protocol/ipa_prf/oprf_padding/mod.rs index bbe092b70..6fe861e2f 100644 --- a/ipa-core/src/protocol/ipa_prf/oprf_padding/mod.rs +++ b/ipa-core/src/protocol/ipa_prf/oprf_padding/mod.rs @@ -582,7 +582,7 @@ mod tests { } #[test] - #[ignore] + #[ignore = "manual execution only"] pub fn table_of_padding_parameters() { // see output https://docs.google.com/spreadsheets/d/1N0WEUkarP_6nd-7W8O9r-Xurh9OImESgAC1Jd_6OfWw/edit?gid=0#gid=0 let epsilon_values = [0.01, 0.1, 1.0, 5.0, 10.0]; From 609150d3a32fadea3388d8dcd7a630f959202dd9 Mon Sep 17 00:00:00 2001 From: Alex Koshelev Date: Thu, 12 Feb 2026 16:06:12 -0800 Subject: [PATCH 4/7] Fix doc comment formatting Add backticks around code references in doc comments to satisfy `clippy::doc_markdown`. --- ipa-core/src/helpers/transport/handler.rs | 2 +- ipa-core/src/net/server/mod.rs | 2 +- ipa-core/src/protocol/basics/reshare.rs | 4 ++-- ipa-core/src/protocol/context/dzkp_validator.rs | 4 ++-- ipa-core/src/protocol/context/validator.rs | 2 +- ipa-core/src/query/processor.rs | 8 ++++---- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/ipa-core/src/helpers/transport/handler.rs b/ipa-core/src/helpers/transport/handler.rs index b36a1e3b4..1da76dd2f 100644 --- a/ipa-core/src/helpers/transport/handler.rs +++ b/ipa-core/src/helpers/transport/handler.rs @@ -177,7 +177,7 @@ pub enum Error { } /// Trait for custom-handling different request types made against MPC helper parties. -/// There is a limitation for RPITIT that traits can't be made object-safe, hence the use of async_trait +/// There is a limitation for RPITIT that traits can't be made object-safe, hence the use of `async_trait` #[async_trait] pub trait RequestHandler: Send + Sync { /// Handle the incoming request with metadata/headers specified in [`Addr`] and body encoded as diff --git a/ipa-core/src/net/server/mod.rs b/ipa-core/src/net/server/mod.rs index 6e4a538df..f4fc7af0e 100644 --- a/ipa-core/src/net/server/mod.rs +++ b/ipa-core/src/net/server/mod.rs @@ -670,7 +670,7 @@ mod e2e_tests { } /// Ensures that server tracks number of requests it received and emits a corresponding metric. - /// In order for this test not to be flaky, we rely on tokio::test macro to set up a + /// In order for this test not to be flaky, we rely on `tokio::test` macro to set up a /// new runtime per test (which it currently does) and set up metric recorders per thread (done /// by this test). It is also tricky to make it work in a multi-threaded environment - I haven't /// tested that, so better to stick with default behavior of tokio:test macro diff --git a/ipa-core/src/protocol/basics/reshare.rs b/ipa-core/src/protocol/basics/reshare.rs index 57a14c3d0..9f9b46bfb 100644 --- a/ipa-core/src/protocol/basics/reshare.rs +++ b/ipa-core/src/protocol/basics/reshare.rs @@ -29,8 +29,8 @@ use crate::{ /// 1. While calculating for a helper, we call pseudo random secret sharing (prss) to get random values which match /// with those generated by other helpers (say `rand_left`, `rand_right`) /// `to_helper.left` knows `rand_left` (named r1) and `to_helper.right` knows `rand_right` (named r0) -/// 2. `to_helper.left` calculates part1 = (a1 + a2) - r2 = Same as (input.left() + input.right()) - r1 from helper POV -/// `to_helper.right` calculates part2 = (a3 - r3) = Same as (input.left() - r0) from helper POV +/// 2. `to_helper.left` calculates part1 = (a1 + a2) - r2 = Same as (`input.left()` + `input.right()`) - r1 from helper POV +/// `to_helper.right` calculates part2 = (a3 - r3) = Same as (`input.left()` - r0) from helper POV /// 3. `to_helper.left` and `to_helper.right` exchange their calculated shares /// 4. Everyone sets their shares /// `to_helper.left` = (part1 + part2, `rand_left`) = (part1 + part2, r1) diff --git a/ipa-core/src/protocol/context/dzkp_validator.rs b/ipa-core/src/protocol/context/dzkp_validator.rs index 4ede763c0..4a732ad14 100644 --- a/ipa-core/src/protocol/context/dzkp_validator.rs +++ b/ipa-core/src/protocol/context/dzkp_validator.rs @@ -684,7 +684,7 @@ pub trait DZKPValidator: Send + Sync { fn context(&self) -> Self::Context; /// Sets the validator's total number of records field. This is required when using - /// the validate_record API, if it wasn't already set on the context used to create + /// the `validate_record` API, if it wasn't already set on the context used to create /// the validator. fn set_total_records>(&mut self, total_records: T); @@ -860,7 +860,7 @@ impl<'a, B: ShardBinding> DZKPValidator for MaliciousDZKPValidator<'a, B> { } /// `is_verified` checks that there are no `MultiplicationInputs` that have not been verified. - /// This function is called by drop() to ensure that the validator is safe to be dropped. + /// This function is called by `drop()` to ensure that the validator is safe to be dropped. /// /// ## Errors /// Errors when there are `MultiplicationInputs` that have not been verified. diff --git a/ipa-core/src/protocol/context/validator.rs b/ipa-core/src/protocol/context/validator.rs index ff5c7b7c7..41a11bd43 100644 --- a/ipa-core/src/protocol/context/validator.rs +++ b/ipa-core/src/protocol/context/validator.rs @@ -423,7 +423,7 @@ mod tests { /// This is the simplest arithmetic circuit that allows us to test all of the pieces of this validator /// A - /// \ - /// Mult_Gate -> A*B + /// `Mult_Gate` -> A*B /// / /// B - /// diff --git a/ipa-core/src/query/processor.rs b/ipa-core/src/query/processor.rs index c8312316c..a53f595fc 100644 --- a/ipa-core/src/query/processor.rs +++ b/ipa-core/src/query/processor.rs @@ -1128,9 +1128,9 @@ mod tests { use crate::{helpers::query::CompareStatusRequest, protocol::QueryId}; /// * From the standpoint of leader shard in Helper 1 - /// * On query_status + /// * On `query_status` /// - /// The min state should be returned. In this case, if I, as leader, am in AwaitingInputs + /// The min state should be returned. In this case, if I, as leader, am in `AwaitingInputs` /// state and shards report that they are further ahead (Completed and Running), then my /// state is returned. #[tokio::test] @@ -1188,9 +1188,9 @@ mod tests { } /// * From the standpoint of leader shard in Helper 1 - /// * On query_status + /// * On `query_status` /// - /// If one of my shards hasn't received the query yet (NoSuchQuery) the leader should + /// If one of my shards hasn't received the query yet (`NoSuchQuery`) the leader should /// return an error despite other shards returning their status #[tokio::test] #[should_panic( From 96f828f0c91b53de201833f24180761826bd59a3 Mon Sep 17 00:00:00 2001 From: Alex Koshelev Date: Thu, 12 Feb 2026 16:06:20 -0800 Subject: [PATCH 5/7] Apply miscellaneous clippy fixes - Use `.clone()` instead of `.to_string()` on `String` derefs (`clippy::implicit_clone`) - Replace `is_some()`/`unwrap()` with `if let Some` (`clippy::unnecessary_unwrap`) - Remove needless `return` statements (`clippy::needless_return`) - Derive `Default` instead of manual impl (`clippy::derivable_impls`) - Use `std::slice::from_ref` instead of `&[x.clone()]` (`clippy::cloned_ref_to_slice_refs`) --- ipa-core/benches/ct/dzkp.rs | 2 +- ipa-core/src/cli/clientconf.rs | 2 +- ipa-core/src/cli/config_parse.rs | 6 +++--- ipa-core/src/cli/keygen.rs | 10 +++++----- ipa-core/src/cli/playbook/mod.rs | 2 +- ipa-core/src/cli/test_setup.rs | 2 +- ipa-core/src/error.rs | 9 ++------- ipa-core/src/helpers/stream/chunks.rs | 2 +- 8 files changed, 15 insertions(+), 20 deletions(-) diff --git a/ipa-core/benches/ct/dzkp.rs b/ipa-core/benches/ct/dzkp.rs index 2e5b87997..294d45af1 100644 --- a/ipa-core/benches/ct/dzkp.rs +++ b/ipa-core/benches/ct/dzkp.rs @@ -66,7 +66,7 @@ fn benchmark_proof(c: &mut Criterion) { (a, b) }, |(a, b): (Vec, Vec)| async move { - TestWorld::default() + let _ = TestWorld::default() .malicious((a.into_iter(), b.into_iter()), |ctx, (a, b)| async move { let batch_size = non_zero_prev_power_of_two( TARGET_PROOF_SIZE / usize::try_from(BA::BITS).unwrap(), diff --git a/ipa-core/src/cli/clientconf.rs b/ipa-core/src/cli/clientconf.rs index 453eb2f09..67104d7f0 100644 --- a/ipa-core/src/cli/clientconf.rs +++ b/ipa-core/src/cli/clientconf.rs @@ -63,7 +63,7 @@ pub fn setup(args: ConfGenArgs) -> Result<(), BoxError> { .map(|(id, (host, (port, shard_port)))| { let id: u8 = u8::try_from(id).unwrap() + 1; HelperClientConf { - host: host.to_string(), + host: host.clone(), port, shard_port, tls_cert_file: args.keys_dir.helper_tls_cert(id), diff --git a/ipa-core/src/cli/config_parse.rs b/ipa-core/src/cli/config_parse.rs index 1efdf0e67..26e8068c1 100644 --- a/ipa-core/src/cli/config_parse.rs +++ b/ipa-core/src/cli/config_parse.rs @@ -361,8 +361,8 @@ pub fn sharded_server_from_toml_str( let url = myself.url.to_string(); let pos = url.rfind(':'); let port = shard_port.expect("Shard port should be set"); - let new_url = if pos.is_some() { - format!("{}{port}", &url[..=pos.unwrap()]) + let new_url = if let Some(pos) = pos { + format!("{}{port}", &url[..=pos]) } else { format!("{}:{port}", &url) }; @@ -374,7 +374,7 @@ pub fn sharded_server_from_toml_str( }; Ok((mpc_network, shard_network)) } else { - return Err(Error::MissingShardUrls(missing_urls)); + Err(Error::MissingShardUrls(missing_urls)) } } diff --git a/ipa-core/src/cli/keygen.rs b/ipa-core/src/cli/keygen.rs index a0ddec88f..dd3afbdb5 100644 --- a/ipa-core/src/cli/keygen.rs +++ b/ipa-core/src/cli/keygen.rs @@ -103,11 +103,11 @@ pub fn keygen_tls(args: &KeygenArgs, rng: &mut R) -> Result< fn keygen_matchkey(args: &KeygenArgs, mut rng: &mut R) -> Result<(), BoxError> { let keypair = crate::hpke::KeyPair::r#gen(&mut rng); - if args.mk_public_key.is_some() && args.mk_private_key.is_some() { - create_new(args.mk_public_key.as_ref().unwrap())? - .write_all(hex::encode(keypair.pk_bytes()).as_bytes())?; - create_new(args.mk_private_key.as_ref().unwrap())? - .write_all(hex::encode(keypair.sk_bytes()).as_bytes())?; + if let (Some(mk_public_key), Some(mk_private_key)) = + (args.mk_public_key.as_ref(), args.mk_private_key.as_ref()) + { + create_new(mk_public_key)?.write_all(hex::encode(keypair.pk_bytes()).as_bytes())?; + create_new(mk_private_key)?.write_all(hex::encode(keypair.sk_bytes()).as_bytes())?; } Ok(()) diff --git a/ipa-core/src/cli/playbook/mod.rs b/ipa-core/src/cli/playbook/mod.rs index 513a91cc9..95cb5bdb0 100644 --- a/ipa-core/src/cli/playbook/mod.rs +++ b/ipa-core/src/cli/playbook/mod.rs @@ -222,7 +222,7 @@ pub async fn make_clients( // Note: This closure is only called when the selected action uses clients. let clients = IpaHttpClient::from_conf(&IpaRuntime::current(), &network, &ClientIdentity::None); - wait_for_servers(wait, &[clients.clone()]).await; + wait_for_servers(wait, std::slice::from_ref(&clients)).await; (clients, network) } diff --git a/ipa-core/src/cli/test_setup.rs b/ipa-core/src/cli/test_setup.rs index b6ef5cdeb..3639b6f2e 100644 --- a/ipa-core/src/cli/test_setup.rs +++ b/ipa-core/src/cli/test_setup.rs @@ -174,7 +174,7 @@ fn make_client_configs( keygen(&keygen_args)?; Ok(HelperClientConf { - host: localhost.to_string(), + host: localhost.clone(), port: mpc_port, shard_port, tls_cert_file: keygen_args.tls_cert, diff --git a/ipa-core/src/error.rs b/ipa-core/src/error.rs index f9d55bcee..ee9700f8f 100644 --- a/ipa-core/src/error.rs +++ b/ipa-core/src/error.rs @@ -22,13 +22,14 @@ use crate::{ /// * `ipa::ff::Error`, for finite field routines /// * `ipa::net::Error`, for the HTTP transport /// * `ipa::app::Error`, for the report collector query APIs -#[derive(Error, Debug)] +#[derive(Error, Debug, Default)] pub enum Error { #[error("already exists")] AlreadyExists, #[error("already setup")] AlreadySetup, #[error("internal")] + #[default] Internal, #[error("invalid id found: {0}")] InvalidId(String), @@ -106,12 +107,6 @@ pub enum Error { DuplicateBytes(usize), } -impl Default for Error { - fn default() -> Self { - Self::Internal - } -} - impl Error { #[must_use] pub fn path_parse_error(source: &str) -> Error { diff --git a/ipa-core/src/helpers/stream/chunks.rs b/ipa-core/src/helpers/stream/chunks.rs index 4f7d150ee..5df92c8de 100644 --- a/ipa-core/src/helpers/stream/chunks.rs +++ b/ipa-core/src/helpers/stream/chunks.rs @@ -277,7 +277,7 @@ where ChunkType::Partial(remainder_len), )) } else { - return None; + None } } } From eea1f22f3778b514414fdfe51f72c6459b9cf77a Mon Sep 17 00:00:00 2001 From: Alex Koshelev Date: Fri, 13 Feb 2026 09:33:27 -0800 Subject: [PATCH 6/7] Trigger CI re-run From e9308145474da0a95425f88937208f3534bffaa4 Mon Sep 17 00:00:00 2001 From: Alex Koshelev Date: Fri, 13 Feb 2026 12:40:28 -0800 Subject: [PATCH 7/7] Fix sporadic keygen_confgen test failure (LSan exit code 23) The metrics collector thread spawned by install_collector() was never joined before process exit. CollectorHandle::drop() only logged a warning if the thread was still running, but did not stop or join it. When running tests with `-Z sanitizer=leak`, LSan's atexit handler races against the still-running metrics thread. If LSan runs its leak check before the thread processes the channel disconnect and terminates, it sees the thread's live allocations as leaks and exits with code 23. Fix by properly shutting down the metrics thread in Drop. --- ipa-core/src/cli/metric_collector.rs | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/ipa-core/src/cli/metric_collector.rs b/ipa-core/src/cli/metric_collector.rs index 2cf678f6c..e1fae888d 100644 --- a/ipa-core/src/cli/metric_collector.rs +++ b/ipa-core/src/cli/metric_collector.rs @@ -8,9 +8,9 @@ use tokio::runtime::Builder; /// Holds a reference to metrics controller and producer pub struct CollectorHandle { - thread_handle: JoinHandle<()>, + thread_handle: Option>, /// This will be used once we start consuming metrics - controller: MetricsCollectorController, + controller: Option, producer: MetricsProducer, } @@ -26,16 +26,23 @@ pub fn install_collector() -> io::Result { tracing::info!("Metrics engine is enabled"); Ok(CollectorHandle { - thread_handle: handle, - controller, + thread_handle: Some(handle), + controller: Some(controller), producer, }) } impl Drop for CollectorHandle { fn drop(&mut self) { - if !thread::panicking() && !self.thread_handle.is_finished() { - tracing::warn!("Metrics thread is still running"); + if thread::panicking() { + return; // avoid potential deadlock during panic unwind + } + // Drop controller first to disconnect the command channel. + // This causes the collector thread's event_loop to exit. + drop(self.controller.take()); + // Wait for the collector thread to finish. + if let Some(handle) = self.thread_handle.take() { + let _ = handle.join(); } } } @@ -61,7 +68,12 @@ impl CollectorHandle { /// If metrics is not initialized #[must_use] pub fn scrape_metrics(&self) -> Vec { - let mut store = self.controller.snapshot().expect("Metrics must be set up"); + let mut store = self + .controller + .as_ref() + .expect("Metrics must be set up") + .snapshot() + .expect("Metrics snapshot failed"); let mut buff = Vec::new(); store.export(&mut buff);