[BUG] Serial TTY Signal Regression

[pryre]

Description / Steps to reproduce the issue

Summary

I believe there's been a behavior regression caused by #14104, may also be related to #16334.

Impact

Behavior:

• Before: sending a SIGINT (via CONFIG_TTY_SIGINT) gets received by arbitrary programs run through NSH
• After: sending a SIGINT (via CONFIG_TTY_SIGINT) gets processed, but is ignored due to a group mismatch on sig_dispatch.c:736.

The actual issue seems to be with the check of group == this_task()->group, where a program in sleep passes off to the scheduler (? sorry, I'm not familiar with the specifics), meaning that this_task()->group gets the group from g_idletcb which does not match the program sleeping in the foreground.

Testing

Reverting changes from nxsig_tgkill() to nxsig_kill() allow signals to be received by a program in usleep().

Using the nucleo-l432kc:nsh configuration, with the following config changes:

CONFIG_TTY_SIGINT=y
CONFIG_TTY_SIGINT_CHAR=0x03

CONFIG_SIG_DEFAULT=y
CONFIG_SIG_SIGKILL_ACTION=y
CONFIG_SIG_SIGSTOP_ACTION=y
CONFIG_GROUP_KILL_CHILDREN_TIMEOUT_MS=-1

CONFIG_EXAMPLES_HELLO=y
CONFIG_EXAMPLES_HELLO_PROGNAME="hello"
CONFIG_EXAMPLES_HELLO_PRIORITY=100
CONFIG_EXAMPLES_HELLO_STACKSIZE=2048

Example implementation using hello example app:

/****************************************************************************
 * apps/examples/hello/hello_main.c
 *
 * SPDX-License-Identifier: Apache-2.0
 *
 * Licensed to the Apache Software Foundation (ASF) under one or more
 * contributor license agreements.  See the NOTICE file distributed with
 * this work for additional information regarding copyright ownership.  The
 * ASF licenses this file to you under the Apache License, Version 2.0 (the
 * "License"); you may not use this file except in compliance with the
 * License.  You may obtain a copy of the License at
 *
 *   http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
 * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
 * License for the specific language governing permissions and limitations
 * under the License.
 *
 ****************************************************************************/

/****************************************************************************
 * Included Files
 ****************************************************************************/

#include <nuttx/config.h>
#include <stdio.h>
#include <signal.h>

/****************************************************************************
 * Public Functions
 ****************************************************************************/

volatile bool _ok;
void _handler(int sig) {
  _ok = false;
}

/****************************************************************************
 * hello_main
 ****************************************************************************/

int main(int argc, FAR char *argv[])
{
  printf("Hello, World...\n");
  
  _ok = true;
  signal(SIGINT, _handler);

  while(_ok) {
    usleep(10*1000*1000);
  }

  printf("Bye!\n");

  return 0;
}

Output pressing CTRL-C before changes during the usleep():

Hello, World...
Bye!

Output pressing CTRL-C before changes during the usleep():

Hello, World...

Where the program never exits.

On which OS does this issue occur?

[OS: Linux]

What is the version of your OS?

OpenSUSE Linux 6.15.8-1.0.2.sr20250701-default

NuttX Version

12.10.0

Issue Architecture

[Arch: arm]

Issue Area

[Area: Posix]

Host information

No response

Verification

• I have verified before submitting the report.

[pryre]

I'm not sure if it's a related issue, but the desired behavior would be for usleep() to follow the POSIX convention, where an interrupted call would return -1 with errno set to EINTR:

RETURN VALUE         [top](https://man7.org/linux/man-pages/man3/usleep.3.html#top_of_page)

       The usleep() function returns 0 on success.  On error, -1 is
       returned, with [errno](https://man7.org/linux/man-pages/man3/errno.3.html) set to indicate the error.

ERRORS         [top](https://man7.org/linux/man-pages/man3/usleep.3.html#top_of_page)

       EINTR  Interrupted by a signal; see [signal(7)](https://man7.org/linux/man-pages/man7/signal.7.html).

       EINVAL usec is greater than or equal to 1000000.  (On systems
              where that is considered an error.)

I was not able to get this behavior on either side of the change listed.

[xiaoxiang781216]

@jlaitine could you look at this regression.

[jlaitine]

Sure. I believe this bug actually originates from #13530

Use of this_task()->group looks just wrong, as this is called also from interrupt handler (this_task might be whatever....). tid however identifies the thread correctly.

Perhaps it should simply be stcb->group()... I'll make a quick patch so let's see.

I am not sure, however, why the signal is delivered to thread instead of group in this case (The PR #14104 )

[jlaitine]

See: #16939

Still, I wonder what is the reason for #14104 ? I believe the signal in this case should be handled by the group, i.e. by any thread in that process?

[pryre]

From my limited understanding of the POSIX spec, it seems that the thread that handles an external signal (e.g. from kill) is intended to be arbitrary in general. It's the. Up to get program to mask or direct the signal from there. To me, this seems like the right response to a keyboard interrupt (or in this case from the tty serial driver).

Perhaps the goal was to have it be more predictable in that the main thread of a process would always receive it instead of an arbitrary one?

It's also strange that nxsig_tgkill() doesn't seem to be used elsewhere (unless my GitHub search results are lying).

[jlaitine]

From my limited understanding of the POSIX spec, it seems that the thread that handles an external signal (e.g. from kill) is intended to be arbitrary in general. It's the. Up to get program to mask or direct the signal from there. To me, this seems like the right response to a keyboard interrupt (or in this case from the tty serial driver).

Perhaps the goal was to have it be more predictable in that the main thread of a process would always receive it instead of an arbitrary one?

It's also strange that nxsig_tgkill() doesn't seem to be used elsewhere (unless my GitHub search results are lying).

Yes, sending signals like ctrl+c to a specific thread is (imho) wrong; imagine e.g. a case where process' main thread creates worker threads, and joins them (wait until they exit). This is not uncommon, and sending a signal to the main thread only leads (probably?) to signal delivery only when the process is exiting for some other reason (main thread gets to run again).

Those signals should always be delivered to group, not to a specific thread.

Thats why I don't understand the PR #14104 - it is not strictly against posix, imho, because signal may be handled by any thread within the process. But I think it is still wrong...