xiaoxiang781216 commented on code in PR #8740:
URL: https://github.com/apache/nuttx/pull/8740#discussion_r1133043639
##########
sched/signal/sig_dispatch.c:
##########
@@ -373,11 +373,11 @@ int nxsig_tcbdispatch(FAR struct tcb_s *stcb, siginfo_t
*info)
#endif
{
#ifdef CONFIG_LIB_SYSCALL
- /* If the thread is in syscall, schedule the sigaction here */
+ /* If the thread is in syscall, queue the sigaction here */
- if ((stcb->flags & TCB_FLAG_SYSCALL) != 0)
+ if (masked == 0)
Review Comment:
> I think the original implementation worked like this, so this would mean
to revert #2851 ?
>
It have been done.
> What about the signal action in case 1. ? Even if a signal is masked, the
task might have injected an action / signal handler for the signal or there
might be a default action. When the procsigmask changes I think the signal
handler should be called, i.e. the signal action should be queued even in case
1. Or am I missing something ?
Yes, the signal should be queue regardless the signal mask setting.
> Just to clarify a bit more, if you take a look into the unmasked signal
handling, the first thing that is done is a signal _action_ is queued. Then if
the task is waiting for a signal it is also unblocked / woken up. This way the
sigunbmask is received by sigwait() and if a signal action handler is installed
by the user, it also gets called. This is logical to me.
>
Normally, the user either install the signal action and unmask the signal,
or mask the signal but call sigwait, it's very rare to use both at the same
time. Another method to receive the signal is signalfd which is added recently:
https://github.com/apache/nuttx/pull/7991.
>
https://github.com/apache/nuttx/blob/fce927123ed243426814f38b6e89b82ac859a7bc/sched/signal/sig_dispatch.c#L434-L475
>
> The masked signal handling is not clear to me. The task is woken up by
this, but no signal action is added anywhere. So the user handler is never
called, even if the sigprocmask changes.
>
>
https://github.com/apache/nuttx/blob/0e4466682888cf32ca661aa04c649a836860a22f/sched/signal/sig_dispatch.c#L389-L426
>
> This is what I don't understand and I think there is an error here, i.e.
depending on whether this is true / false, the signal action is properly queued
or not, and the user handler is either called (when sigprocmask changes) or not.
>
No, it's right behaviour. We don't need do any additional thing except
return directly since it's the expected by the caller of sigwait:
https://man7.org/linux/man-pages/man3/sigwait.3.html
As, I mention before, it's seldom that user call sigwait/signal
action/signalfd at the same time.
> ```
> if (stcb->task_state == TSTATE_WAIT_SIG &&
> (masked == 0 ||
> nxsig_ismember(&stcb->sigwaitmask, info->si_signo)))
> ```
>
> Please note that `nxsig_add_pendingsignal` is needed in this case to
deliver the signal action if/when the sigprocmask changes.
>
> This is the odd part to me, this branch just wakes the task up but the
signal handler is never called, regardless of any system call involvement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
