Skip to content

Commit 6cb2bc8

Browse files
committed
fix(signal): deduplicate process-group delivery
DragonOS links every thread into PIDTYPE_PGID, unlike Linux which links only thread-group leaders. Process-group delivery therefore sent one process-directed signal per thread, allowing a blocked sibling to leave a duplicate shared pending SIGHUP that later terminated pty_test. Select physical leader entries, deduplicate transient leaders by stable TGID identity, and prefer the live leader during de_thread handoff. Add a realtime-signal regression test that proves one killpg call creates exactly one pending instance per thread group. Signed-off-by: longjin <longjin@dragonos.org>
1 parent 675667c commit 6cb2bc8

2 files changed

Lines changed: 81 additions & 2 deletions

File tree

kernel/src/ipc/kill.rs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use crate::ipc::syscall::sys_kill::check_signal_permission_pcb_with_sig;
33
use crate::process::pid::{Pid, PidType};
44
use crate::process::{ProcessControlBlock, ProcessManager, RawPid};
55
use crate::{arch::ipc::signal::Signal, ipc::signal_types::SigCode};
6+
use alloc::collections::BTreeMap;
67
use alloc::sync::Arc;
78
use alloc::vec::Vec;
89
use core::sync::atomic::compiler_fence;
@@ -73,8 +74,28 @@ pub fn send_signal_to_pcb(
7374
/// 参考 https://code.dragonos.org.cn/xref/linux-6.6.21/kernel/signal.c?fi=kill_pgrp#1921
7475
#[inline(never)]
7576
pub fn send_signal_to_pgid(pgid: &Arc<Pid>, sig: Signal) -> Result<usize, SystemError> {
76-
// 先收集进程组中的所有进程,避免在持有锁时调用复杂操作
77-
let tasks: Vec<Arc<ProcessControlBlock>> = pgid.tasks_iter(PidType::PGID).collect();
77+
// Linux only links thread-group leaders into PIDTYPE_PGID. DragonOS
78+
// currently links every thread, so only use physical leader entries and
79+
// group them by their stable TGID identity. TGID grouping also covers the
80+
// brief old/new leader overlap during de_thread().
81+
let mut thread_groups: BTreeMap<usize, Arc<ProcessControlBlock>> = BTreeMap::new();
82+
let physical_tasks: Vec<Arc<ProcessControlBlock>> =
83+
pgid.tasks_iter(PidType::PGID).collect();
84+
for task in physical_tasks {
85+
if !task.is_thread_group_leader() {
86+
continue;
87+
}
88+
let Some(tgid) = task.task_pid_ptr(PidType::TGID) else {
89+
continue;
90+
};
91+
let representative = thread_groups
92+
.entry(Arc::as_ptr(&tgid) as usize)
93+
.or_insert_with(|| task.clone());
94+
if !representative.is_live_thread_group_member() && task.is_live_thread_group_member() {
95+
*representative = task;
96+
}
97+
}
98+
let tasks: Vec<Arc<ProcessControlBlock>> = thread_groups.into_values().collect();
7899

79100
// 如果进程组中没有任何进程,返回 ESRCH
80101
if tasks.is_empty() {

user/apps/tests/dunitest/suites/normal/process_signal_fork.cc

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,15 @@ void* PauseForeverThread(void*) {
7373
return nullptr;
7474
}
7575

76+
void* ReadyPauseThread(void* arg) {
77+
int ready_fd = *static_cast<int*>(arg);
78+
WriteByteOrExit(ready_fd, 'R');
79+
for (;;) {
80+
pause();
81+
}
82+
return nullptr;
83+
}
84+
7685
void RunMultithreadedSignalChild(int ready_fd) {
7786
pthread_t threads[3] {};
7887
for (pthread_t& thread : threads) {
@@ -175,6 +184,55 @@ TEST(ProcessSignalFork, ProcessDirectedSigkillKillsStoppedChildWithPendingStopEv
175184
EXPECT_EQ(SIGKILL, WTERMSIG(status));
176185
}
177186

187+
TEST(ProcessSignalFork, ProcessGroupSignalIsDeliveredOncePerThreadGroup) {
188+
pid_t child = fork();
189+
ASSERT_GE(child, 0) << "fork failed: errno=" << errno << " (" << strerror(errno) << ")";
190+
191+
if (child == 0) {
192+
if (setsid() < 0) {
193+
_exit(120);
194+
}
195+
196+
const int signal = SIGRTMIN;
197+
sigset_t blocked;
198+
sigemptyset(&blocked);
199+
sigaddset(&blocked, signal);
200+
if (sigprocmask(SIG_BLOCK, &blocked, nullptr) != 0) {
201+
_exit(121);
202+
}
203+
204+
int ready_pipe[2] = {-1, -1};
205+
if (pipe(ready_pipe) != 0) {
206+
_exit(122);
207+
}
208+
pthread_t thread {};
209+
if (pthread_create(&thread, nullptr, ReadyPauseThread, &ready_pipe[1]) != 0
210+
|| !ReadByte(ready_pipe[0])) {
211+
_exit(123);
212+
}
213+
214+
if (kill(0, signal) != 0) {
215+
_exit(124);
216+
}
217+
218+
timespec timeout {};
219+
timeout.tv_nsec = 10 * 1000 * 1000;
220+
if (sigtimedwait(&blocked, nullptr, &timeout) != signal) {
221+
_exit(125);
222+
}
223+
errno = 0;
224+
if (sigtimedwait(&blocked, nullptr, &timeout) != -1 || errno != EAGAIN) {
225+
_exit(126);
226+
}
227+
_exit(0);
228+
}
229+
230+
int status = 0;
231+
ASSERT_EQ(child, waitpid(child, &status, 0));
232+
ASSERT_TRUE(WIFEXITED(status)) << "child status=" << status;
233+
EXPECT_EQ(0, WEXITSTATUS(status));
234+
}
235+
178236
TEST(ProcessSignalFork, ProcessDirectedSigkillKillsMultithreadedChild) {
179237
int ready_pipe[2] = {-1, -1};
180238
ASSERT_EQ(0, pipe(ready_pipe)) << "pipe failed: " << strerror(errno);

0 commit comments

Comments
 (0)