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 https://github.com/libuv/libuv/commit/b9421d70665352138557d2d2338656a38ac70691, 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!

[slavamuravey]

@bnoordhuis What do you think, what if we apply changes from reverted PR for everything except of macos? And when issues will be fixed in macos, we apply changes for it too.

[bnoordhuis]

Libuv's goal is to abstract away platform differences. If something can't be made to work in a cross-platform way, we don't do it.

macOS (and Linux and Windows) are popular enough that we can't just shrug and say "too bad, doesn't work here."