macos: restoring signal disposition doesn't work as flags are not propagated

[santigimeno]

Take this very basic example:

#include <stdio.h>
#include <signal.h>
#include <stdlib.h>

void handler(int signum) {
    printf("Signal handler called with signal %d\n", signum);
}

int main() {
    struct sigaction sa, old_sa;

    // Set a signal handler with SA_RESETHAND flag
    sa.sa_handler = handler;
    sigemptyset(&sa.sa_mask);
    sa.sa_flags = SA_RESETHAND;
    if (sigaction(SIGINT, &sa, NULL) == -1) {
        perror("sigaction");
        exit(1);
    }

    // Set a second signal handler without SA_RESETHAND flag
    sa.sa_flags = 0;
    if (sigaction(SIGINT, &sa, &old_sa) == -1) {
        perror("sigaction");
        exit(1);
    }

    // Check whether the SA_RESETHAND flag is actually propagated
    if (old_sa.sa_flags & SA_RESETHAND) {
        printf("SA_RESETHAND was set in the old signal handler\n");
    } else {
        printf("SA_RESETHAND was not set in the old signal handler\n");
    }

    return 0;
}

Running this on linux produces the correct output:

$ SA_RESETHAND was set in the old signal handler

Whereas on macos (arm64) it doesn't:

$ SA_RESETHAND was not set in the old signal handler

I observed this while running current v1.x with the node.js test suite and caused some signal tests to fail. They started failing after b9421d7, though I don't know if there's much we can do from the libuv side as it seems a macos issue. Thoughts?

[vtjnash]

There appear to be some mistakes in that commit, which don't entirely negate the fact this appears to be a kernel bug, but is making it work poorly. In particular

static void uv__sigaction_set(int signum, struct sigaction *sa) {
  uv__sigactions.acts[signum] = *sa;
  uv__sigactions.acts_presented_flags[signum] = 1;
}

looks like it is missing a conditional set:

static void uv__sigaction_set(int signum, struct sigaction *sa) {
  if (uv__sigactions.acts_presented_flags[signum] == 0)
    uv__sigactions.acts[signum] = *sa;
  uv__sigactions.acts_presented_flags[signum] = 1;
}

since libuv may re-register its own sigaction to change from SA_RESETHAND => 0, and that would overwrite the correct info for the old sigaction, leading to libuv not correctly unregistering itself later

[bnoordhuis]

@slavamuravey fyi

I don't know if there's much we can do from the libuv side as it seems a macos issue

Yes... Guess there isn't much we can do here except maybe add a regression test to ensure we don't forget this tidbit of macos lore and merge another attempt in the future.

[slavamuravey]

@bnoordhuis @vtjnash @santigimeno
Perhaps we should store previous sigaction data ourselves and not rely on the kernel?
Here is an example based on @santigimeno's example:

#define _GNU_SOURCE
#include <stdio.h>
#include <signal.h>
#include <stdlib.h>

void handler(int signum) {
    printf("Signal handler called with signal %d\n", signum);
}

int main() {
    struct sigaction sa, old_sa;

    // Set a signal handler with SA_RESETHAND flag
    sa.sa_handler = handler;
    sigemptyset(&sa.sa_mask);
    sa.sa_flags = SA_RESETHAND;
    if (sigaction(SIGINT, &sa, NULL) == -1) {
        perror("sigaction");
        exit(1);
    }
    // Here we remember previous sigaction data
    old_sa = sa;

    // Set a second signal handler without SA_RESETHAND flag
    sa.sa_flags = 0;
    if (sigaction(SIGINT, &sa, NULL) == -1) {
        perror("sigaction");
        exit(1);
    }

    // Check whether the SA_RESETHAND flag is actually propagated
    if (old_sa.sa_flags & SA_RESETHAND) {
        printf("SA_RESETHAND was set in the old signal handler\n");
    } else {
        printf("SA_RESETHAND was not set in the old signal handler\n");
    }

    return 0;
}

[bnoordhuis]

Perhaps we should store previous sigaction data ourselves and not rely on the kernel?

I don't think that would work reliably. A third party (like another library) may have changed the signal handler before libuv gets to it.

Longstanding libuv philosophy is that features should either work always or never, not sometimes. Users can code defensively around the first two but not the last one.

[slavamuravey]

@bnoordhuis Ok, then we should wait for changes in macos. After macos behavior will be the same as in linux for our case, we will be able to merge reverted PR again?

[bnoordhuis]

Yes, but that may be a while because with macos we generally support the latest-1 or latest-2 releases (e.g. macos 11 and up right now.)

[slavamuravey]

Got it, thanks!