Skip to content

Commit ea11082

Browse files
authored
fix: avoid deadlock in nested shell calls (#9245)
* fix: avoid deadlock in nested shell calls * cleanup
1 parent 7587eb5 commit ea11082

File tree

2 files changed

+21
-9
lines changed

2 files changed

+21
-9
lines changed

crates/common/src/io/macros.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,15 +132,23 @@ macro_rules! sh_eprintln {
132132
#[macro_export]
133133
macro_rules! __sh_dispatch {
134134
($f:ident $fmt:literal $($args:tt)*) => {
135-
$crate::Shell::$f(&mut *$crate::Shell::get(), ::core::format_args!($fmt $($args)*))
135+
$crate::__sh_dispatch!(@impl $f &mut *$crate::Shell::get(), $fmt $($args)*)
136136
};
137137

138138
($f:ident $shell:expr, $($args:tt)*) => {
139-
$crate::Shell::$f($shell, ::core::format_args!($($args)*))
139+
$crate::__sh_dispatch!(@impl $f $shell, $($args)*)
140140
};
141141

142142
($f:ident $($args:tt)*) => {
143-
$crate::Shell::$f(&mut *$crate::Shell::get(), ::core::format_args!($($args)*))
143+
$crate::__sh_dispatch!(@impl $f &mut *$crate::Shell::get(), $($args)*)
144+
};
145+
146+
// Ensure that the global shell lock is held for as little time as possible.
147+
// Also avoids deadlocks in case of nested calls.
148+
(@impl $f:ident $shell:expr, $($args:tt)*) => {
149+
match ::core::format_args!($($args)*) {
150+
fmt => $crate::Shell::$f($shell, fmt),
151+
}
144152
};
145153
}
146154

@@ -168,6 +176,11 @@ mod tests {
168176
sh_eprintln!("eprintln")?;
169177
sh_eprintln!("eprintln {}", "arg")?;
170178

179+
sh_println!("{:?}", {
180+
sh_println!("hi")?;
181+
"nested"
182+
})?;
183+
171184
Ok(())
172185
}
173186

crates/common/src/io/shell.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,9 @@ impl Shell {
186186
}
187187
}
188188

189-
/// Get a static reference to the global shell.
189+
/// Acquire a lock to the global shell.
190190
///
191-
/// Initializes the global shell with the default values if it has not been set yet.
191+
/// Initializes it with the default values if it has not been set yet.
192192
pub fn get() -> impl DerefMut<Target = Self> + 'static {
193193
GLOBAL_SHELL.get_or_init(Default::default).lock().unwrap_or_else(PoisonError::into_inner)
194194
}
@@ -200,10 +200,9 @@ impl Shell {
200200
/// Panics if the global shell has already been set.
201201
#[track_caller]
202202
pub fn set(self) {
203-
if GLOBAL_SHELL.get().is_some() {
204-
panic!("attempted to set global shell twice");
205-
}
206-
GLOBAL_SHELL.get_or_init(|| Mutex::new(self));
203+
GLOBAL_SHELL
204+
.set(Mutex::new(self))
205+
.unwrap_or_else(|_| panic!("attempted to set global shell twice"))
207206
}
208207

209208
/// Sets whether the next print should clear the current line and returns the previous value.

0 commit comments

Comments
 (0)