Skip to content

Commit 59b8104

Browse files
martinvonzByron
authored andcommitted
fix!: mark gix::interrupt::init_handler() as unsafe
The passed `interrupt()` argument will be called from a signal handler, so that needs to be documented and the call sites need to state that they fulfill the contract. Thanks to @Manishearth for pointing this out.
1 parent d77bc0e commit 59b8104

File tree

3 files changed

+20
-9
lines changed

3 files changed

+20
-9
lines changed

gix/src/interrupt.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,10 @@ mod init {
8989
///
9090
/// It will abort the process on second press and won't inform the user about this behaviour either as we are unable to do so without
9191
/// deadlocking even when trying to write to stderr directly.
92-
pub fn init_handler(
92+
///
93+
/// SAFETY: `interrupt()` will be called from a signal handler. See `signal_hook::low_level::register()` for details about.
94+
#[allow(unsafe_code)]
95+
pub unsafe fn init_handler(
9396
grace_count: usize,
9497
interrupt: impl Fn() + Send + Sync + Clone + 'static,
9598
) -> io::Result<Deregister> {

src/plumbing/main.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,14 @@ pub fn main() -> Result<()> {
136136
let auto_verbose = !progress && !args.no_verbose;
137137

138138
let should_interrupt = Arc::new(AtomicBool::new(false));
139-
gix::interrupt::init_handler(1, {
140-
let should_interrupt = Arc::clone(&should_interrupt);
141-
move || should_interrupt.store(true, Ordering::SeqCst)
142-
})?;
139+
#[allow(unsafe_code)]
140+
unsafe {
141+
// SAFETY: The closure doesn't use mutexes or memory allocation, so it should be safe to call from a signal handler.
142+
gix::interrupt::init_handler(1, {
143+
let should_interrupt = Arc::clone(&should_interrupt);
144+
move || should_interrupt.store(true, Ordering::SeqCst)
145+
})?;
146+
}
143147

144148
match cmd {
145149
Subcommands::Status(crate::plumbing::options::status::Platform {

src/porcelain/main.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,14 @@ pub fn main() -> Result<()> {
1818
time::util::local_offset::set_soundness(time::util::local_offset::Soundness::Unsound);
1919
}
2020
let should_interrupt = Arc::new(AtomicBool::new(false));
21-
gix::interrupt::init_handler(1, {
22-
let should_interrupt = Arc::clone(&should_interrupt);
23-
move || should_interrupt.store(true, Ordering::SeqCst)
24-
})?;
21+
#[allow(unsafe_code)]
22+
unsafe {
23+
// SAFETY: The closure doesn't use mutexes or memory allocation, so it should be safe to call from a signal handler.
24+
gix::interrupt::init_handler(1, {
25+
let should_interrupt = Arc::clone(&should_interrupt);
26+
move || should_interrupt.store(true, Ordering::SeqCst)
27+
})?;
28+
}
2529
let trace = false;
2630
let verbose = !args.quiet;
2731
let progress = args.progress;

0 commit comments

Comments
 (0)