Search code examples
c++c++17signals

Is std::atomic or volatile necessary when setting variables from a signal handler


#include <unistd.h>

#include <csignal>
#include <exception>
#include <functional>
#include <iostream>

std::function<void(int)> g_signalHandler;

void signalWrapper(const int a) { g_signalHandler(a); }

int main() {
    bool abort = false;
    g_signalHandler = [&abort](const int) {
        std::cout << "Abort" << std::endl;
        abort = true;
    };

    struct ::sigaction signalAction = {};
    signalAction.sa_handler = signalWrapper;

    if (
        ::sigaction(SIGHUP,  &signalAction, nullptr) != 0 ||
        ::sigaction(SIGINT,  &signalAction, nullptr) != 0 ||
        ::sigaction(SIGTERM, &signalAction, nullptr) != 0
    )
        throw std::exception();

    // Is it guaranteed that the new value of abort will be seen here without
    // the need of std::atomic<bool> or volatile bool?
    while (!abort)
        ::sleep(1);

    return 0;
}

Assume a single threaded program, C++17, and Linux

To the best of my knowledge using a normal bool is adequate for while (!abort) and std::atomic<bool> / volatile bool is not necessary. I would like to confirm that this is always true and that I don't have to worry about the compiler optimizing out reading the value


Solution

  • To the best of my knowledge using a normal bool is adequate for while (!abort) and std::atomic / volatile bool is not necessary. I would like to confirm that this is always true and that I don't have to worry about the compiler optimizing out reading the value

    No, that is wrong and will cause a data race, which is always undefined behavior.

    To demonstrate how such undefined behavior could reasonably manifest: The compiler may see that abort is non-atomic and that ::sleep does not imply any synchronization. Therefore it can conclude that abort will never change if it is initially false, since that would cause a data race with undefined behavior, and either get rid of the loop check completely or keep abort in a register without ever reloading it from memory. In either case, your loop will never terminate.

    volatile bool is also wrong. volatile is not sufficient according to the requirements of the standard for signal handlers.

    volatile does guarantee that each read and write in the code will actually translate to a load and store, however volatile does not make any guarantee on atomicity of these loads/stores. (However, specific compiler/platform combinations may make such guarantees, so that volatile may be used to implement atomic operations.)

    The only non-local variables you are guaranteed to be able to access with defined behavior in a signal handler are (non-thread-local)

    • objects of type volatile std::atomic_sig_t (and accessed through volatile glvalue); only if shared (without other synchronization) exclusively between the signal handler and the thread on which the signal handler runs, no other threads
    • lock-free atomic objects; std::atomic_flag is guaranteed lock-free; which ones of the std::atomic specializations are lock-free is platform-dependent and can be checked with the is_lock_free and is_always_lock_free members of std::atomic

    Furthermore, library functions are by default not safe to use in a signal handler either. There are only specific exceptions in the C/C++/POSIX standards. Specifically std::function and stream IO as in std::cout << are not safe in signal handlers and also cause undefined behavior.

    See https://en.cppreference.com/w/cpp/utility/program/signal for the requirements that the C++ standard imposes on signal handlers in order for them to have well-defined behavior according to the standard.

    And see https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html#tag_16_04_03 for a list of async-signal-safe functions in POSIX environments.