Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 874ca28

Browse files
yellowhatterYuanYuYuan
andauthoredMar 24, 2025
Nolocal close (#1632)
* Make background close builder nolocal * make close builders internally-public * Update lib.rs * do not use flume channel s it is not nolocal * fix exports * switch runtime to Net * Make close more atexit-safe * add tests for wait() in close * Add important note * Update Cargo.toml and atexit.rs * Code format * fix clippy * Update Cargo.lock * - review fixes - get rid of async-channel (use tokio channel instead until it is nolocal) - improve testing coverage (test background close to be nolocal) * code format * fix feature set * disable session_close_in_atexit test on 1.85 as std has regression rust-lang/rust#138696 * fix tokio executor blockage * Paic with message describing nolocal thread spawning error * spelling * Handle nolocal errors more properly * refactor: replace `mpsc::channel` with `oneshot::channel` and address the `block_in_place` issue (#26) --------- Co-authored-by: Yuyuan Yuan <az6980522@gmail.com>
1 parent 95b15dd commit 874ca28

File tree

7 files changed

+286
-15
lines changed

7 files changed

+286
-15
lines changed
 

‎Cargo.lock

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎zenoh/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ once_cell = { workspace = true }
124124

125125
[dev-dependencies]
126126
tokio = { workspace = true }
127+
libc = { workspace = true }
127128

128129
[build-dependencies]
129130
rustc_version = { workspace = true }

‎zenoh/build.rs

+8
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,12 @@ fn main() {
1717
"cargo:rustc-env=RUSTC_VERSION={}",
1818
version_meta.short_version_string
1919
);
20+
21+
let version = rustc_version::version().unwrap();
22+
if version >= rustc_version::Version::parse("1.85.0").unwrap()
23+
&& version <= rustc_version::Version::parse("1.85.1").unwrap()
24+
{
25+
// https://github.com/rust-lang/rust/issues/138696
26+
println!("cargo:rustc-cfg=nolocal_thread_not_available");
27+
}
2028
}

‎zenoh/src/api/builders/close.rs

+87-12
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ use std::{
1919
};
2020

2121
use async_trait::async_trait;
22+
#[cfg(all(feature = "unstable", feature = "internal"))]
23+
use tokio::sync::oneshot::Receiver;
2224
use zenoh_core::{Resolvable, Wait};
2325
use zenoh_result::ZResult;
2426
use zenoh_runtime::ZRuntime;
@@ -73,7 +75,35 @@ impl<TCloseable: Closeable> Resolvable for CloseBuilder<TCloseable> {
7375

7476
impl<TCloseable: Closeable> Wait for CloseBuilder<TCloseable> {
7577
fn wait(self) -> Self::To {
76-
ZRuntime::Application.block_in_place(self.into_future())
78+
let future = self.into_future();
79+
match tokio::runtime::Handle::try_current() {
80+
Ok(_) => {
81+
tracing::trace!("tokio TLS available, closing closeable directly");
82+
ZRuntime::Net.block_in_place(future)
83+
}
84+
Err(e) if e.is_missing_context() => {
85+
tracing::trace!("tokio TLS is just missing, closing closeable directly");
86+
ZRuntime::Net.block_in_place(future)
87+
}
88+
Err(_) => {
89+
#[cfg(nolocal_thread_not_available)]
90+
panic!("Close when thread-local storage is unavailable (typically in atexit()) does not work for this Rust 1.85..1.85.1, see https://github.com/rust-lang/rust/issues/138696");
91+
92+
#[cfg(not(nolocal_thread_not_available))]
93+
{
94+
let evaluate = move || {
95+
// NOTE: tracing logger also panics if used inside atexit() handler!!!
96+
tracing::trace!(
97+
"tokio TLS NOT available, closing closeable in separate thread"
98+
);
99+
ZRuntime::Net.block_in_place(future)
100+
};
101+
std::thread::spawn(evaluate)
102+
.join()
103+
.expect("Error spawning atexit-safe thread")
104+
}
105+
}
106+
}
77107
}
78108
}
79109

@@ -99,19 +129,13 @@ impl<TCloseable: Closeable> IntoFuture for CloseBuilder<TCloseable> {
99129

100130
#[cfg(all(feature = "unstable", feature = "internal"))]
101131
/// A builder for close operations running in background
102-
// NOTE: `Closeable` is only pub(crate) because it is zenoh-internal trait, so we don't
103-
// care about the `private_bounds` lint in this particular case.
104132
#[doc(hidden)]
105-
#[allow(private_bounds)]
106133
pub struct BackgroundCloseBuilder<TOutput: Send + 'static> {
107134
inner: Pin<Box<dyn Future<Output = TOutput> + Send>>,
108135
}
109136

110137
#[cfg(all(feature = "unstable", feature = "internal"))]
111138
#[doc(hidden)]
112-
// NOTE: `Closeable` is only pub(crate) because it is zenoh-internal trait, so we don't
113-
// care about the `private_bounds` lint in this particular case.
114-
#[allow(private_bounds)]
115139
impl<TOutput: Send + 'static> BackgroundCloseBuilder<TOutput> {
116140
fn new(inner: Pin<Box<dyn Future<Output = TOutput> + Send>>) -> Self {
117141
Self { inner }
@@ -120,13 +144,13 @@ impl<TOutput: Send + 'static> BackgroundCloseBuilder<TOutput> {
120144

121145
#[cfg(all(feature = "unstable", feature = "internal"))]
122146
impl<TOutput: Send + 'static> Resolvable for BackgroundCloseBuilder<TOutput> {
123-
type To = tokio::task::JoinHandle<TOutput>;
147+
type To = NolocalJoinHandle<TOutput>;
124148
}
125149

126150
#[cfg(all(feature = "unstable", feature = "internal"))]
127151
impl<TOutput: Send + 'static> Wait for BackgroundCloseBuilder<TOutput> {
128152
fn wait(self) -> Self::To {
129-
ZRuntime::Application.block_in_place(self.into_future())
153+
ZRuntime::Net.block_in_place(self.into_future())
130154
}
131155
}
132156

@@ -135,10 +159,61 @@ impl<TOutput: Send + 'static> IntoFuture for BackgroundCloseBuilder<TOutput> {
135159
type Output = <Self as Resolvable>::To;
136160
type IntoFuture = Pin<Box<dyn Future<Output = <Self as IntoFuture>::Output> + Send>>;
137161

138-
// NOTE: yes, we need to return a future that returns JoinHandle
139-
#[allow(clippy::async_yields_async)]
140162
fn into_future(self) -> Self::IntoFuture {
141-
Box::pin(async move { ZRuntime::Application.spawn(self.inner) }.into_future())
163+
Box::pin(
164+
async move {
165+
let (tx, rx) = tokio::sync::oneshot::channel();
166+
ZRuntime::Net.spawn(async move {
167+
if tx.send(self.inner.await).is_err() {
168+
panic!("BackgroundCloseBuilder: critical error sending the result");
169+
}
170+
});
171+
NolocalJoinHandle::new(rx)
172+
}
173+
.into_future(),
174+
)
175+
}
176+
}
177+
178+
#[cfg(all(feature = "unstable", feature = "internal"))]
179+
#[doc(hidden)]
180+
pub struct NolocalJoinHandle<TOutput: Send + 'static> {
181+
rx: Receiver<TOutput>,
182+
}
183+
184+
#[cfg(all(feature = "unstable", feature = "internal"))]
185+
impl<TOutput: Send + 'static> NolocalJoinHandle<TOutput> {
186+
fn new(rx: Receiver<TOutput>) -> Self {
187+
Self { rx }
188+
}
189+
}
190+
191+
#[cfg(all(feature = "unstable", feature = "internal"))]
192+
impl<TOutput: Send + 'static> Resolvable for NolocalJoinHandle<TOutput> {
193+
type To = TOutput;
194+
}
195+
196+
#[cfg(all(feature = "unstable", feature = "internal"))]
197+
impl<TOutput: Send + 'static> Wait for NolocalJoinHandle<TOutput> {
198+
fn wait(self) -> Self::To {
199+
ZRuntime::Net.block_in_place(self.into_future())
200+
}
201+
}
202+
203+
#[cfg(all(feature = "unstable", feature = "internal"))]
204+
impl<TOutput: Send + 'static> IntoFuture for NolocalJoinHandle<TOutput> {
205+
type Output = <Self as Resolvable>::To;
206+
type IntoFuture = Pin<Box<dyn Future<Output = <Self as IntoFuture>::Output> + Send>>;
207+
208+
fn into_future(self) -> Self::IntoFuture {
209+
Box::pin(
210+
async move {
211+
self.rx
212+
.await
213+
.expect("NolocalJoinHandle: critical error receiving the result")
214+
}
215+
.into_future(),
216+
)
142217
}
143218
}
144219

‎zenoh/src/lib.rs

+6
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,12 @@ compile_error!(
477477

478478
#[zenoh_macros::internal]
479479
pub mod internal {
480+
#[zenoh_macros::unstable]
481+
pub mod builders {
482+
pub mod close {
483+
pub use crate::api::builders::close::{BackgroundCloseBuilder, NolocalJoinHandle};
484+
}
485+
}
480486
pub mod traits {
481487
pub use crate::api::builders::sample::{
482488
EncodingBuilderTrait, QoSBuilderTrait, SampleBuilderTrait, TimestampBuilderTrait,

‎zenoh/tests/atexit.rs

+165
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
//
2+
// Copyright (c) 2025 ZettaScale Technology
3+
//
4+
// This program and the accompanying materials are made available under the
5+
// terms of the Eclipse Public License 2.0 which is available at
6+
// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
7+
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
8+
//
9+
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
10+
//
11+
// Contributors:
12+
// ZettaScale Zenoh Team, <zenoh@zettascale.tech>
13+
//
14+
15+
#![cfg(feature = "internal_config")]
16+
17+
fn run_in_separate_process(main_name: &str, must_panic: bool) {
18+
let output = std::process::Command::new(std::env::current_exe().unwrap())
19+
.arg(main_name)
20+
.arg("--nocapture")
21+
.arg("--show-output")
22+
.arg("--exact")
23+
.arg("--include-ignored")
24+
.env(
25+
{
26+
match must_panic {
27+
true => "PROBE_PANIC_IN_TEST",
28+
false => "PROBE_NO_PANIC_IN_TEST",
29+
}
30+
},
31+
"true",
32+
)
33+
.output()
34+
.expect("Failed to run test in separate process");
35+
36+
match must_panic {
37+
true => {
38+
assert!(
39+
!output.status.success(),
40+
"Inner test should have been failed"
41+
);
42+
}
43+
false => {
44+
assert!(output.status.success(), "Inner test failed");
45+
}
46+
};
47+
}
48+
49+
#[ignore]
50+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
51+
async fn panic_in_separate_process() {
52+
if std::env::var("PROBE_PANIC_IN_TEST").is_ok() {
53+
panic!("Panic in test because PROBE_PANIC_IN_TEST is set!");
54+
}
55+
}
56+
57+
#[ignore]
58+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
59+
async fn atexit_panic_in_separate_process() {
60+
unsafe { libc::atexit(panic_in_atexit) };
61+
}
62+
63+
extern "C" fn panic_in_atexit() {
64+
if std::env::var("PROBE_PANIC_IN_TEST").is_ok() {
65+
panic!("Panic in atexit because PROBE_PANIC_IN_TEST is set!");
66+
}
67+
}
68+
69+
#[test]
70+
fn panic_is_seen_in_separate_process() {
71+
run_in_separate_process("panic_in_separate_process", true);
72+
}
73+
74+
#[test]
75+
fn panic_is_seen_in_separate_process_atexit() {
76+
run_in_separate_process("atexit_panic_in_separate_process", true);
77+
}
78+
79+
use std::sync::OnceLock;
80+
81+
use zenoh::{Session, Wait};
82+
static SESSION: OnceLock<Session> = OnceLock::new();
83+
84+
#[ignore]
85+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
86+
async fn session_close_in_atexit_main() {
87+
if std::env::var("PROBE_PANIC_IN_TEST").is_ok() {
88+
panic!("Panic in test because PROBE_PANIC_IN_TEST is set!");
89+
}
90+
91+
// Open the sessions
92+
let mut config = zenoh::Config::default();
93+
config
94+
.listen
95+
.endpoints
96+
.set(vec!["tcp/127.0.0.1:19446".parse().unwrap()])
97+
.unwrap();
98+
config.scouting.multicast.set_enabled(Some(false)).unwrap();
99+
100+
let s = zenoh::open(config).await.unwrap();
101+
let _session = SESSION.get_or_init(move || s);
102+
103+
unsafe { libc::atexit(close_session_in_atexit) };
104+
}
105+
106+
extern "C" fn close_session_in_atexit() {
107+
let session = SESSION.get().unwrap();
108+
session.close().wait().unwrap();
109+
}
110+
111+
#[cfg(not(nolocal_thread_not_available))]
112+
#[test]
113+
fn session_close_in_atexit() {
114+
run_in_separate_process("session_close_in_atexit_main", true);
115+
run_in_separate_process("session_close_in_atexit_main", false);
116+
}
117+
118+
#[cfg(all(feature = "unstable", feature = "internal"))]
119+
use std::sync::Mutex;
120+
121+
#[cfg(all(feature = "unstable", feature = "internal"))]
122+
use zenoh::{internal::builders::close::NolocalJoinHandle, Result};
123+
124+
#[cfg(all(feature = "unstable", feature = "internal"))]
125+
static BACKGROUND_SESSION: OnceLock<Session> = OnceLock::new();
126+
#[cfg(all(feature = "unstable", feature = "internal"))]
127+
static CLOSE: OnceLock<Mutex<Option<NolocalJoinHandle<Result<()>>>>> = OnceLock::new();
128+
129+
#[cfg(all(feature = "unstable", feature = "internal"))]
130+
#[ignore]
131+
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
132+
async fn background_session_close_in_atexit_main() {
133+
if std::env::var("PROBE_PANIC_IN_TEST").is_ok() {
134+
panic!("Panic in test because PROBE_PANIC_IN_TEST is set!");
135+
}
136+
137+
// Open the sessions
138+
let mut config = zenoh::Config::default();
139+
config
140+
.listen
141+
.endpoints
142+
.set(vec!["tcp/127.0.0.1:19445".parse().unwrap()])
143+
.unwrap();
144+
config.scouting.multicast.set_enabled(Some(false)).unwrap();
145+
146+
let s = zenoh::open(config).await.unwrap();
147+
let session = BACKGROUND_SESSION.get_or_init(move || s);
148+
149+
let _close = CLOSE.get_or_init(|| Mutex::new(Some(session.close().in_background().wait())));
150+
151+
unsafe { libc::atexit(background_close_session_in_atexit) };
152+
}
153+
154+
#[cfg(all(feature = "unstable", feature = "internal"))]
155+
extern "C" fn background_close_session_in_atexit() {
156+
let mut close = CLOSE.get().unwrap().lock().unwrap();
157+
close.take().unwrap().wait().unwrap();
158+
}
159+
160+
#[cfg(all(feature = "unstable", feature = "internal"))]
161+
#[test]
162+
fn background_session_close_in_atexit() {
163+
run_in_separate_process("background_session_close_in_atexit_main", true);
164+
run_in_separate_process("background_session_close_in_atexit_main", false);
165+
}
There was a problem loading the remainder of the diff.

0 commit comments

Comments
 (0)
Failed to load comments.