drivers/serial: Fix SIGINT not delivered to foreground process on Ctrl-C #18116
Conversation
…l-C. When pressing Ctrl-C, the foreground process did not receive SIGINT and failed to terminate. The serial driver called nxsig_tgkill(-1, dev->pid, signo) from interrupt context. With pid=-1, nxsig_dispatch() was called with thread=true, which requires stcb->group == this_task()->group. However, in interrupt context, this_task() returns the IDLE task, whose group differs from the target process group. This caused the signal dispatch to fail with -ESRCH. Solution: Replace nxsig_tgkill(-1, pid, signo) with nxsig_kill(pid, signo). nxsig_kill() uses thread=false, which routes through group_signal() without the same-group check, allowing signals to be delivered correctly from interrupt context. Impact: - Fixes Ctrl-C signal delivery in serial console - No API changes - Affects serial driver interrupt handling only Testing: Verified on QEMU ARM64 simulator with serial console Signed-off-by: yinshengkai <yinshengkai@bytedance.com>
Gary-Hobson
requested review from
GUIDINGLI,
jerpelea and
xiaoxiang781216
as code owners
github-actions
bot
added
Area: Drivers
|
@Gary-Hobson I didn't know that Ctrl+C should hit a process running in background, but I double checked it on Linux and in fact it worked, but there are a catch: If the process is running in foregroup Ctrl+C will terminate it: When the process is running in backgroup Ctrl+C just stops it, but it still there, see: Are you replicating this behavior, or Ctrl+C will also terminate the process running in background? I think we should replicate the Linux behavior :-) |
|
@cederom do you know if FreeBSD also do like Linux in this case? |
This looks bad idea to use Ctrl+C to kill background jobs, because you can kill many jobs that way accidentally? Ctrl+C generates a signal that is handled by current application only. I can replicate this behavior but this seems bash specific, zsh does not behave this way. Also there is a difference if task is moved to background by Ctrl+Z or launched with You need to Tested in zsh and bash (note I have two top already running as tmux plugins): Now |
|
I will test and verify fix on a real hardware in a free moment just to confirm, thanks @Gary-Hobson and @acassis :-) |
|
@cederom I think Ctrl+C will break only the last job sent to background: In fact this is a good improvement, we just need to guarantee that it is doing the right thing. @Gary-Hobson please test sending two process to background and test to confirm it will only terminate the last one. |
|
Yeah, but its bash only, and Ctrl+C does not kill the background process, you can still bring it back working with |
The current code doesn't terminate any background processes, and as I see it, terminating background processes probably isn't a good idea. Below are the steps for my test:
int main(int argc, FAR char *argv[])
{
printf("Hello, World!!\n");
sleep(30);
return 0;
}
./tools/configure.sh mps2-an500/nsh
make menuconfig
# Add the following two configurations:
CONFIG_SIG_DEFAULT=y
CONFIG_TTY_SIGINT=y
make -jRun the test on QEMU: qemu-system-arm -M mps2-an500 -cpu cortex-m7 -nographic -kernel ./nuttx
nsh> hello
press Ctrl+C
nxtask_exit: hello pid=3,TCB=0x60007410
nsh> |
@Gary-Hobson yes, I think I read background instead foreground and decided to test it on Linux. But I agree it not a good idea to kill a background process using Ctrl+C. I was surprised to see the Linux shell doing it. |
5 participants
Summary
This PR fixes a critical bug where pressing Ctrl-C in a serial console does not deliver SIGINT to the foreground process, preventing proper process termination.
Root Cause:
The serial driver called
nxsig_tgkill(-1, dev->pid, signo)from interrupt context. Withpid=-1,nxsig_dispatch()was invoked withthread=true, which requiresstcb->group == this_task()->group. However, in interrupt context,this_task()returns the IDLE task, whose group differs from the target process group. This caused the signal dispatch to fail with-ESRCH.Changes:
nxsig_tgkill(-1, pid, signo)withnxsig_kill(pid, signo)in serial_dma.cnxsig_tgkill(-1, pid, signo)withnxsig_kill(pid, signo)in serial_io.cThe
nxsig_kill()function usesthread=false, which routes throughgroup_signal()without the same-group check, allowing signals to be delivered correctly from interrupt context.Impact
Testing
Tested on sim and arm64 platforms with serial console configurations.