Skip to content

signal/sig_dispatch: Add signal action, if task is in system call#8740

Merged
masayuki2009 merged 1 commit into
apache:masterfrom
tiiuae:fix_double_sig
Mar 14, 2023
Merged

signal/sig_dispatch: Add signal action, if task is in system call#8740
masayuki2009 merged 1 commit into
apache:masterfrom
tiiuae:fix_double_sig

Conversation

@pussuw
Copy link
Copy Markdown
Contributor

@pussuw pussuw commented Mar 6, 2023

Summary

This fixes regression found in #8605. Only the signal action should be queued when in system call, the signal should not be sent and pended which results in the user app receiving it twice.

Impact

Fix regression from #8605

Testing

icicle:nsh / pnsh + ostest and https://github.com/linux-test-project/ltp/blob/master/testcases/open_posix_testsuite/conformance/interfaces/timer_settime/9-2.c

Comment thread sched/signal/sig_dispatch.c Outdated
@pussuw
Copy link
Copy Markdown
Contributor Author

pussuw commented Mar 6, 2023

There might still be a problem with this fix as well. The syscall entry modifies the user context, and so does up_schedule_sigaction. This needs more testing still.

@pussuw pussuw marked this pull request as draft March 6, 2023 18:55
@pussuw
Copy link
Copy Markdown
Contributor Author

pussuw commented Mar 7, 2023

@xiaoxiang781216 I'll just revert all my changes to get the old functionality back / get rid of the regression. I need to do more homework on how the signal delivery with sigwait() actually works in NuttX to fix this properly.

It seems the critical combination is sigwait() and its friends combined with CONFIG_LIB_SYSCALL. I believe some new functionality is needed to handle this case. The problem seems to be that sigwait() is abandoned at once, if a pending signal is waiting, masked or not.

The _unmasked_ signal action was never added if the task is in system call
and waiting for (a different) signal.

This fixes deliver especially for default signal actions / unmaskable
signals, like SIGTERM.
@pussuw pussuw changed the title signal/sig_dispatch: Fix sending the same signal twice signal/sig_dispatch: Add signal action, if task is in system call Mar 13, 2023
@pussuw
Copy link
Copy Markdown
Contributor Author

pussuw commented Mar 13, 2023

The sigwait test now works with system calls enabled as well, with icicle:pnsh:

spawn_execattrs: Setting policy=2 priority=100 for pid=4
nxsig_tcbdispatch: TCB=0x8008c500 signo=0 code=0 value=0 mask=00000000
clock_gettime: clock_id=0
clock_gettime: Returning tp=(43201225,90000000)

Tclock_gettime: clock_id=0
clock_gettime: Returning tp=(43201225,100000000)
est for value 43201225 sec 180000000 nsec
nxsig_notification: pid=4 signo=14 code=2 sival_ptr=0
nxsig_tcbdispatch: TCB=0x8008c500 signo=14 code=2 value=0 mask=00004000
clock_gettime: clock_id=0
clock_gettime: Returning tp=(43201225,200000000)
Tclock_gettime: clock_id=0
clock_gettime: Returning tp=(43201225,200000000)
imclock_gettime: clock_id=0
clock_gettime: Returning tp=(43201225,210000000)
er expired 43201225 sec 200000000 nsec
Test for value 43201226 sec 200000000 nsec
nxsig_notification: pid=4 signo=14 code=2 sival_ptr=0
nxsig_tcbdispatch: TCB=0x8008c500 signo=14 code=2 value=0 mask=00004000
clock_gettime: clock_id=0
clock_gettime: Returning tp=(43201226,220000000)
Tclock_gettime: clock_id=0
clock_gettime: Returning tp=(43201226,220000000)
imclock_gettime: clock_id=0
clock_gettime: Returning tp=(43201226,230000000)
er expired 43201226 sec 220000000 nsec
Test for value 43201227 sec 250000000 nsec
nxsig_notification: pid=4 signo=14 code=2 sival_ptr=0
nxsig_tcbdispatch: TCB=0x8008c500 signo=14 code=2 value=0 mask=00004000
clock_gettime: clock_id=0
clock_gettime: Returning tp=(43201227,270000000)
Tclock_gettime: clock_id=0
clock_gettime: Returning tp=(43201227,270000000)
imclock_gettime: clock_id=0
clock_gettime: Returning tp=(43201227,280000000)
er expired 43201227 sec 270000000 nsec
Test for value 43201229 sec 270000000 nsec
nxsig_notification: pid=4 signo=14 code=2 sival_ptr=0
nxsig_tcbdispatch: TCB=0x8008c500 signo=14 code=2 value=0 mask=00004000
clock_gettime: clock_id=0
clock_gettime: Returning tp=(43201229,300000000)
Tclock_gettime: clock_id=0
clock_gettime: Returning tp=(43201229,300000000)
imclock_gettime: clock_id=0
clock_gettime: Returning tp=(43201229,310000000)
er expired 43201229 sec 300000000 nsec
Test for value 43201232 sec 300005000 nsec
nxsig_notification: pid=4 signo=14 code=2 sival_ptr=0
nxsig_tcbdispatch: TCB=0x8008c500 signo=14 code=2 value=0 mask=00004000
clock_gettime: clock_id=0
clock_gettime: Returning tp=(43201232,330000000)
Tclock_gettime: clock_id=0
clock_gettime: Returning tp=(43201232,330000000)
imclock_gettime: clock_id=0
clock_gettime: Returning tp=(43201232,340000000)
er expired 43201232 sec 330000000 nsec
Test for value 43201236 sec 330000005 nsec
nxsig_notification: pid=4 signo=14 code=2 sival_ptr=0
nxsig_tcbdispatch: TCB=0x8008c500 signo=14 code=2 value=0 mask=00004000
clock_gettime: clock_id=0
clock_gettime: Returning tp=(43201236,360000000)
Tinxsig_tcbdispatch: TCB=0x8008bde0 signo=17 code=5 value=0 mask=00000000
up_exit: TCB=0x8008c500 exiting
nxsig_tcbdispatch: TCB=0x8008bde0 signo=17 code=5 value=0 mask=00000000
mer expired 43201236 sec 360000000 nsec
Test PASSED

Also killing task when the task is in sleep() works:

task_spawn: name=hello entry=0x800496f0 file_actions=0x800c2230 attr=0x800c2238 argv=0x800c2320
spawn_execattrs: Setting policy=2 priority=100 for pid=5

hello [5:100]
nsh> kill 5
nxsig_tcbdispatch: TCB=0x8008c500 signo=15 code=0 value=0 mask=00000000
nxsig_tcbdispatch: TCB=0x8008c500 signo=15 code=0 value=0 mask=00000000
up_schedule_sigaction: tcb=0x8008c500 sigdeliver=0x80001fbc
up_schedule_sigaction: rtcb=0x8008c500 CURRENT_REGS=0x800c3bf0
up_schedule_sigaction: PC/STATUS Saved: 00000000800501dc/8000000a00004080 New: 0000000080009992/8000000a00005800
riscv_sigdeliver: rtcb=0x8008c500 sigdeliver=0x80001fbc sigpendactionq.head=0x800880d0
nxsig_deliver: Deliver signal 15 to PID 5
nxsig_tcbdispatch: TCB=0x8008bde0 signo=17 code=5 value=0 mask=00000000
up_exit: TCB=0x8008c500 exiting
nxsig_tcbdispatch: TCB=0x8008bde0 signo=17 code=5 value=0 mask=00000000
nsh> ps
PID GROUP PRI POLICY TYPE NPX STATE EVENT SIGMASK STACK USED FILLED COMMAND
0 0 0 FIFO Kthread N-- Ready 00000000 002000 001472 73.6% Idle Task
1 1 224 RR Kthread --- Waiting Semaphore 00000000 004000 000704 17.6% hpwork 0x80082240
2 2 100 RR Kthread --- Waiting Semaphore 00000000 004000 000704 17.6% lpwork 0x80082270
3 3 100 RR Task --- Running 00000000 002992 002336 78.0% nsh_main
nsh>

Hello is simply:

int main(void)
{
  while(true)
    {
      sleep(5);
    }
  return 0;
}

@pussuw pussuw marked this pull request as ready for review March 13, 2023 10:21
@pussuw pussuw requested review from xiaoxiang781216 and removed request for GUIDINGLI and masayuki2009 March 13, 2023 10:21
Copy link
Copy Markdown
Contributor

@pkarashchenko pkarashchenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@masayuki2009 masayuki2009 merged commit 776e2d7 into apache:master Mar 14, 2023
@pussuw pussuw deleted the fix_double_sig branch March 14, 2023 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants