diff --git a/iceoryx2-bb/linux/tests-common/src/epoll_tests.rs b/iceoryx2-bb/linux/tests-common/src/epoll_tests.rs index f29b76e88d..27f1768c4c 100644 --- a/iceoryx2-bb/linux/tests-common/src/epoll_tests.rs +++ b/iceoryx2-bb/linux/tests-common/src/epoll_tests.rs @@ -312,55 +312,41 @@ pub fn blocking_wait_wakes_up_by_trigger() { }).unwrap(); } -// TODO: #1458 +// The global `SignalHandler` claims every non-fatal signal's disposition; an epoll +// subscribed to that same signal must still receive it as `EpollEvent::Signal`. +// TODO: #1898 +// currently failing as the epoll's signalfd is starved by the competing `call_and_fetch(..)` #[ignore] #[test] pub fn signals_can_be_received() { - let _watchdog = Watchdog::new_with_timeout(core::time::Duration::from_secs(1000)); - let (socket_1, _socket_2) = StreamingSocket::create_pair().unwrap(); + let _watchdog = Watchdog::new(); let sut = EpollBuilder::new() .handle_signal(FetchableSignal::UserDefined1) .create() .unwrap(); - let _guard = sut - .add(socket_1.file_descriptor()) - .event_type(EventType::ReadyToRead) - .attach() - .unwrap(); - let callback_was_called = AtomicBool::new(false); - let handle = BarrierHandle::new(); - let barrier = BarrierBuilder::new(2).create(&handle).unwrap(); - thread_scope(|s| { - s.thread_builder().spawn(|| { - barrier.wait(); - assert_that!(sut.blocking_wait(|event| { - if let EpollEvent::Signal(signal) = event { - assert_that!(signal.signal(), eq FetchableSignal::UserDefined1); - assert_that!(signal.origin_uid(), eq User::from_self().unwrap().uid()); - assert_that!(signal.origin_pid(), eq Process::from_self().id()); - } else { - assert_that!(true, eq false); - } - callback_was_called.store(true, Ordering::Relaxed); - }).unwrap(), eq 1); - })?; - - barrier.wait(); - nanosleep(TIMEOUT).unwrap(); - assert_that!(callback_was_called.load(Ordering::Relaxed), eq false); - - while !callback_was_called.load(Ordering::Relaxed) { - SignalHandler::call_and_fetch(|| { - Process::from_self() - .send_signal(FetchableSignal::UserDefined1.into()) - .unwrap(); - }); - } - - // thread should wake up now, if not the watchdog will let the unit test fail - - Ok(()) - }) + // Activate the global handler; its default disposition is what starves the fd. + let _ = SignalHandler::call_and_fetch(|| {}); + + SignalHandler::call_and_fetch(|| { + Process::from_self() + .send_signal(FetchableSignal::UserDefined1.into()) + .unwrap(); + }); + + let got_signal = AtomicBool::new(false); + sut.timed_wait( + |event| { + if let EpollEvent::Signal(signal) = event { + assert_that!(signal.signal(), eq FetchableSignal::UserDefined1); + assert_that!(signal.origin_uid(), eq User::from_self().unwrap().uid()); + assert_that!(signal.origin_pid(), eq Process::from_self().id()); + got_signal.store(true, Ordering::Relaxed); + } + }, + core::time::Duration::from_millis(200), + ) .unwrap(); + + assert_that!(got_signal.load(Ordering::Relaxed), eq true); } diff --git a/iceoryx2-bb/linux/tests-common/src/signal_fd_tests.rs b/iceoryx2-bb/linux/tests-common/src/signal_fd_tests.rs index a6b9c12972..6e8d74bfef 100644 --- a/iceoryx2-bb/linux/tests-common/src/signal_fd_tests.rs +++ b/iceoryx2-bb/linux/tests-common/src/signal_fd_tests.rs @@ -19,14 +19,17 @@ use iceoryx2_bb_posix::clock::nanosleep; use iceoryx2_bb_posix::thread::thread_scope; use iceoryx2_bb_posix::{ process::Process, - signal::{FetchableSignal, SignalHandler}, + signal::{FetchableSignal, NonFatalFetchableSignal, SignalHandler}, signal_set::FetchableSignalSet, user::User, }; use iceoryx2_bb_testing::{assert_that, watchdog::Watchdog}; use iceoryx2_bb_testing_macros::test; -// TODO: #1458 +// A signal subscribed on a signalfd must be delivered to the fd. Today the +// global SignalHandler claims every non-fatal signal first and the fd is +// starved. +// TODO: #1898 #[ignore] #[test] fn registered_signal_can_be_try_read() { @@ -35,21 +38,75 @@ fn registered_signal_can_be_try_read() { signals.add(FetchableSignal::UserDefined1); let sut = SignalFdBuilder::new(signals).create_non_blocking().unwrap(); - loop { - SignalHandler::call_and_fetch(|| { - Process::from_self() - .send_signal(FetchableSignal::UserDefined1.into()) - .unwrap(); - }); + // activate the global handler, currently starving the fd + let _ = SignalHandler::call_and_fetch(|| {}); - let signal = sut.try_read().unwrap(); - if let Some(signal) = signal { - assert_that!(signal.signal(), eq FetchableSignal::UserDefined1); - assert_that!(signal.origin_pid(), eq Process::from_self().id()); - assert_that!(signal.origin_uid(), eq User::from_self().unwrap().uid()); + SignalHandler::call_and_fetch(|| { + Process::from_self() + .send_signal(FetchableSignal::UserDefined1.into()) + .unwrap(); + }); + + let mut received = None; + for _ in 0..100 { + if let Some(signal) = sut.try_read().unwrap() { + received = Some(signal); break; } + nanosleep(core::time::Duration::from_millis(2)).ok(); } + + let received = received.unwrap(); + assert_that!(received.signal(), eq FetchableSignal::UserDefined1); + assert_that!(received.origin_pid(), eq Process::from_self().id()); + assert_that!(received.origin_uid(), eq User::from_self().unwrap().uid()); +} + +// Regression guard for #1898: dropping a SignalFd that subscribed to +// UserDefined1 must not leave that signal monopolized away from the global +// handler. After the fd is released the handler must observe UserDefined1 +// again. +#[test] +fn dropped_signal_fd_restores_handler_visibility() { + let _watchdog = Watchdog::new(); + let _ = SignalHandler::call_and_fetch(|| {}); + + { + let mut signals = FetchableSignalSet::new_empty(); + signals.add(FetchableSignal::UserDefined1); + let _sut = SignalFdBuilder::new(signals).create_non_blocking().unwrap(); + } // fd dropped here + + let observed = SignalHandler::call_and_fetch(|| { + Process::from_self() + .send_signal(FetchableSignal::UserDefined1.into()) + .unwrap(); + nanosleep(core::time::Duration::from_millis(2)).ok(); + }); + + assert_that!(observed, eq Some(NonFatalFetchableSignal::UserDefined1)); +} + +// Regression guard for #1898: while a SignalFd owns UserDefined1 the +// global handler must still observe an unrelated signal (Continue). Owning one +// signal must not suppress delivery of a different, unowned signal. +#[test] +fn signal_fd_does_not_mask_unsubscribed_signal() { + let _watchdog = Watchdog::new(); + let _ = SignalHandler::call_and_fetch(|| {}); + + let mut signals = FetchableSignalSet::new_empty(); + signals.add(FetchableSignal::UserDefined1); + let _sut = SignalFdBuilder::new(signals).create_non_blocking().unwrap(); + + let observed = SignalHandler::call_and_fetch(|| { + Process::from_self() + .send_signal(FetchableSignal::Continue.into()) + .unwrap(); + nanosleep(core::time::Duration::from_millis(2)).ok(); + }); + + assert_that!(observed, eq Some(NonFatalFetchableSignal::Continue)); } #[test] diff --git a/iceoryx2-bb/posix/tests-common/src/signal_tests.rs b/iceoryx2-bb/posix/tests-common/src/signal_tests.rs index ad9626b28c..2d765839c6 100644 --- a/iceoryx2-bb/posix/tests-common/src/signal_tests.rs +++ b/iceoryx2-bb/posix/tests-common/src/signal_tests.rs @@ -176,25 +176,62 @@ pub fn call_and_fetch_works() { assert_that!(result, eq Some(NonFatalFetchableSignal::Interrupt)); } -// TODO: #1458 -#[ignore] +// When calling `call_and_fetch` we must not get signals from unrelated threads +// Currently fails due to contention on the singleton `LAST_SIGNAL` latch +// TODO #1898 #[test] -pub fn call_and_fetch_with_registered_handler_works() { +#[ignore] +pub fn call_and_fetch_does_not_observe_unrelated_concurrent_signal() { test_requires!(POSIX_SUPPORT_ADVANCED_SIGNAL_HANDLING); let _watchdog = Watchdog::new(); + let _test = TestFixture::new(); - let test = TestFixture::new(); + let probe_delivered = AtomicI32::new(0); + let window_open = AtomicI32::new(0); + thread_scope(|s| { + // send a signal to ourselves in a new thread + s.thread_builder() + .spawn(|| { + while window_open.load(Ordering::SeqCst) == 0 { + nanosleep(Duration::from_micros(50)).ok(); + } + let tid = unsafe { posix::pthread_self() }; + unsafe { posix::pthread_kill(tid, posix::SIGUSR1) }; + probe_delivered.store(1, Ordering::SeqCst); + }) + .expect("failed to spawn thread"); - let _guard = - SignalHandler::register(FetchableSignal::UserDefined1, &TestFixture::signal_callback); + let result = SignalHandler::call_and_fetch(|| { + // wait for the other thread to complete before we exit the closure + window_open.store(1, Ordering::SeqCst); + assert_that!( + || { probe_delivered.load(Ordering::SeqCst) }, + eq 1, + before Watchdog::default() + ); + }); + + // our `result` must not capture the signal the spawned thread sent to itself + assert_that!(result, eq None); + + Ok(()) + }) + .expect("failed to execute thread scope"); +} + +// We can send signals to ourselves when uncontested +#[test] +pub fn call_and_fetch_observes_signal_directed_at_calling_thread() { + test_requires!(POSIX_SUPPORT_ADVANCED_SIGNAL_HANDLING); + let _watchdog = Watchdog::new(); + let _test = TestFixture::new(); let result = SignalHandler::call_and_fetch(|| { - Process::from_self().send_signal(Signal::UserDefined1).ok(); - nanosleep(TIMEOUT).ok(); + let tid = unsafe { posix::pthread_self() }; + unsafe { posix::pthread_kill(tid, posix::SIGUSR1) }; }); assert_that!(result, eq Some(NonFatalFetchableSignal::UserDefined1)); - test.verify(NonFatalFetchableSignal::UserDefined1, 1); } #[test] @@ -358,3 +395,54 @@ pub fn termination_requested_with_interrupt_works() { ); assert_that!(SignalHandler::termination_requested(), eq false); } + +#[cfg(feature = "std")] +fn completes_within( + deadline: Duration, + op: impl FnOnce() -> T + Send + 'static, +) -> Option { + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let _ = tx.send(op()); + }); + rx.recv_timeout(deadline).ok() +} + +#[cfg(feature = "std")] +static CALLBACK_FED: AtomicI32 = AtomicI32::new(0); + +#[cfg(feature = "std")] +fn mark_callback(_: FetchableSignal) { + CALLBACK_FED.store(1, Ordering::SeqCst); +} + +// capturing an op that raises the same signal a callback is registered +// for must not deadlock, and both consumers must observe it +// TODO #1898 +#[cfg(feature = "std")] +#[ignore] +#[test] +pub fn call_and_fetch_with_registered_callback_completes() { + test_requires!(POSIX_SUPPORT_ADVANCED_SIGNAL_HANDLING); + let _watchdog = Watchdog::new(); + let _test = TestFixture::new(); + + // register a callback for USR1 + let callback_guard = SignalHandler::register(FetchableSignal::UserDefined1, &mark_callback); + std::mem::forget(callback_guard); + + // contend with the above callback + let result = completes_within(Duration::from_secs(2), || { + SignalHandler::call_and_fetch(|| { + let tid = unsafe { posix::pthread_self() }; + unsafe { posix::pthread_kill(tid, posix::SIGUSR1) }; + }) + .ok_or(()) + .expect("Failed to retrieve signal after `call_and_fetch") + }) + .ok_or(()) + .expect("Failed to complete within given timeout"); + + assert_that!(result, eq NonFatalFetchableSignal::UserDefined1); + assert_that!(CALLBACK_FED.load(Ordering::SeqCst), eq 1); +}