Skip to content

Commit 5c36253

Browse files
chore: add warning to block_on
Should this become an unsafe function? Signed-off-by: Tiago Castro <tiagolobocastro@gmail.com>
1 parent d5b5d44 commit 5c36253

File tree

3 files changed

+36
-38
lines changed

3 files changed

+36
-38
lines changed

io-engine/src/bdev/nexus/nexus_bdev.rs

+28-20
Original file line numberDiff line numberDiff line change
@@ -1373,30 +1373,38 @@ impl<'n> BdevOps for Nexus<'n> {
13731373
return;
13741374
}
13751375

1376-
let self_ptr = unsafe { unsafe_static_ptr(&self) };
1377-
1378-
Reactor::block_on(async move {
1379-
let self_ref = unsafe { &mut *self_ptr };
1380-
1381-
// TODO: double-check interaction with rebuild job logic
1382-
// TODO: cancel rebuild jobs?
1383-
let n = self_ref.children.iter().filter(|c| c.is_opened()).count();
1384-
1385-
if n > 0 {
1386-
warn!(
1387-
"{:?}: {} open children remain(s), closing...",
1388-
self_ref, n
1389-
);
1376+
let online_children =
1377+
self.children.iter().filter(|c| c.is_opened()).count();
1378+
// TODO: This doesn't seem possible to happen at this stage, but seems
1379+
// we should still try to handle this in separate future since
1380+
// we're handling it here anyway as a block_on is not safe to
1381+
// use for running production code.
1382+
if online_children > 0 {
1383+
let self_ptr = unsafe { unsafe_static_ptr(&self) };
1384+
Reactor::block_on(async move {
1385+
let self_ref = unsafe { &mut *self_ptr };
1386+
1387+
// TODO: double-check interaction with rebuild job logic
1388+
// TODO: cancel rebuild jobs?
1389+
let n =
1390+
self_ref.children.iter().filter(|c| c.is_opened()).count();
1391+
1392+
if n > 0 {
1393+
warn!(
1394+
"{:?}: {} open children remain(s), closing...",
1395+
self_ref, n
1396+
);
13901397

1391-
for child in self_ref.children.iter() {
1392-
if child.is_opened() {
1393-
child.close().await.ok();
1398+
for child in self_ref.children.iter() {
1399+
if child.is_opened() {
1400+
child.close().await.ok();
1401+
}
13941402
}
13951403
}
1396-
}
13971404

1398-
self_ref.children.clear();
1399-
});
1405+
self_ref.children.clear();
1406+
});
1407+
}
14001408

14011409
self.as_mut().unregister_io_device();
14021410
unsafe {

io-engine/src/core/reactor.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -362,8 +362,15 @@ impl Reactor {
362362
task
363363
}
364364

365-
/// spawn a future locally on the current core block until the future is
365+
/// Spawns a future locally on the current core block until the future is
366366
/// completed. The master core is used.
367+
/// # Warning
368+
/// This code should only be used for testing and not running production!
369+
/// This is because when calling block_on from a thread_poll callback, we
370+
/// may be leaving messages behind, which can lead to timeouts etc...
371+
/// A work-around to make this safe could be to potentially "pull" the
372+
/// messages which haven't been polled, and poll them here before
373+
/// proceeding to re-poll via thread_poll again.
367374
pub fn block_on<F, R>(future: F) -> Option<R>
368375
where
369376
F: Future<Output = R> + 'static,

io-engine/src/grpc/mod.rs

-17
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use futures::channel::oneshot::Receiver;
22
use nix::errno::Errno;
33
pub use server::MayastorGrpcServer;
44
use std::{
5-
error::Error,
65
fmt::{Debug, Display},
76
future::Future,
87
time::Duration,
@@ -158,22 +157,6 @@ macro_rules! spdk_submit {
158157

159158
pub type GrpcResult<T> = std::result::Result<Response<T>, Status>;
160159

161-
/// call the given future within the context of the reactor on the first core
162-
/// on the init thread, while the future is waiting to be completed the reactor
163-
/// is continuously polled so that forward progress can be made
164-
pub fn rpc_call<G, I, L, A>(future: G) -> Result<Response<A>, tonic::Status>
165-
where
166-
G: Future<Output = Result<I, L>> + 'static,
167-
I: 'static,
168-
L: Into<Status> + Error + 'static,
169-
A: 'static + From<I>,
170-
{
171-
Reactor::block_on(future)
172-
.unwrap()
173-
.map(|r| Response::new(A::from(r)))
174-
.map_err(|e| e.into())
175-
}
176-
177160
/// Submit rpc code to the primary reactor.
178161
pub fn rpc_submit<F, R, E>(
179162
future: F,

0 commit comments

Comments
 (0)