[#1898] Add test coverage for signal-isolation behavior

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);
 }

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]

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<T: Send + 'static>(
+    deadline: Duration,
+    op: impl FnOnce() -> T + Send + 'static,
+) -> Option<T> {
+    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);
+}