2

The application is multi-threaded. Inside main(), I register the signal handler for SIGUSR1:

// Global variable to indicate whether directory's 
// content needs to be reloaded
bool reload_dir = 0;

int main (int argc, char *argv[])
{
    ...

    signal(SIGUSR1, sigusr1_handler);

    ...


    RunServer(arg1, ...);
    return 0;
}

Signal handler:

static void
sigusr1_handler (int signo __unused)
{
    reload_dir = 1;
    return;
}

The following function (which is called from main) is only executed by the main thread:

void
RunServer (arg1, ...)
{
    // do some stuffs
    ...

    while (cond != true) {
        sleep(1);
    }

    server_exit();
}

Now, when the SIGUSR1 is caught (by any thread including the main thread), I'm setting the reload_dir variable to 1. And in RunServer(), I'm reloading the directory based on that value. However, I'm also resetting the global variable reload_dir to 0 to avoid loading the directory repeatedly indefinitely. And this setting reload_dir to 0 will introduce a race.

Since we should not use locks or mutex in a signal handler, how can I achieve this without majorly changing the existing application design.

void
RunServer (arg1, ...)
{
    // do some stuffs
    ...

    while (cond != true) {
        if (reload_dir) {
           // reset reload_dir to avoid loading repeatedly indefinitely 
           reload_dir = 0;   // Race condition?
           dir_reinit();
        }
        sleep(1);
    }

    server_exit();
}
void
  • 338
  • 5
  • 19
  • 1
    You might be looking for atomic variables. You should be aware that you can handle signals in your main loop by using signalfd, which does not have this problem. – user253751 Feb 20 '20 at 12:53
  • @user253751, This application (which is single-sourced) runs on both FreeBSD11 and Linux based OS. So, I doubt if signalfd can be leveraged in FreeBSD. – void Feb 20 '20 at 13:00
  • Perhaps you could block signals on the server thread (so that only the main thread can receive signals, this is a good idea anyway), and then set a flag in the signal handler which the main loop picks up. Note that `sleep` may be interrupted by a signal. – user253751 Feb 20 '20 at 13:04
  • Sorry if it wasn't clear via the code sample, but main thread is nothing but the server thread. All other threads are created dynamically (at various other places in the code) based on the number of incoming requests to the application. – void Feb 20 '20 at 13:10
  • try to check https://www.bogotobogo.com/cplusplus/C11/7_C11_Thread_Sharing_Memory.php – Build Succeeded Feb 20 '20 at 13:14
  • Even if `signalfd` isn't available both Linux and BSD have pipes. Create a pipe and use the [self-pipe trick](https://cr.yp.to/docs/selfpipe.html). – G.M. Feb 20 '20 at 13:14
  • @user253751 Also, will just the use of atomic variable in place of the existing global variable, solve the race condition? – void Feb 20 '20 at 13:15
  • @Mannoj, That doesn't answer this question. This question is in the context of signals and not about mutex/locks. – void Feb 20 '20 at 13:18
  • FreeBSD has something equivalent to Linux's `signalfd`, I believe it's called `kqueue`. I have some portable code that uses both. – Sam Varshavchik Feb 20 '20 at 13:19
  • 1
    __unused is a reserved identifier. – eerorika Feb 20 '20 at 13:27
  • @eerorika Yes. Implementations are allowed to define that identifier. C_user5's implementation has defined this identifier and s/he is using it. – user253751 Feb 20 '20 at 14:01
  • 1
    @user253751 By implementation, I mean the implementation of the C++ language. If C_user5 is writing a standard library, then the usage is fine. But then their problem is that they also use non-reserved identifiers. You must pick one or the other depending on whether you are implementer or user of the language. – eerorika Feb 20 '20 at 14:04
  • @eerorika Why do you think that C_user5 defined this identifier? – user253751 Feb 20 '20 at 14:05
  • @user253751 I see no reason to suspect otherwise. Sure, they could technically be asking about a program written by their colleague. But that's irrelevant to the usage of reserved identifier. – eerorika Feb 20 '20 at 14:08
  • @eerorika Why do you think the identifier is defined as part of this program, and not the standard library? – user253751 Feb 20 '20 at 14:10
  • @user253751 Because there is no reason to assume otherwise. Do you think that `sigusr1_handler` is a standard function? Which standard specifies it? Is `reload_dir` a standard identifier as well in your opinion? – eerorika Feb 20 '20 at 14:12
  • 1
    The __unused macro (which is in fact expanded to the __attribute__((unused)) GCC attribute) is used here to tell the compiler "don't warn me if I don't use this variable". – void Feb 20 '20 at 14:19
  • @eerorika ... do you think that `sigusr1_handler` defines `__unused`? – user253751 Feb 20 '20 at 14:44
  • @C_user5 *The __unused macro (which is in fact expanded to the __attribute__((unused)) GCC attribute) is used here to tell the compiler "don't warn me if I don't use this variable".* The [portable way to do that is to simply cast the argument to `void`](https://stackoverflow.com/questions/34288844/what-does-casting-to-void-really-do), as in `( void ) signo;`. – Andrew Henle Feb 20 '20 at 15:32

1 Answers1

1

Block SIGUSR1 with pthread_sigmask before any threads are spawned, so that all threads inherit that mask. Then, in main(), use sigtimedwait instead of sleep in your main loop to check if USR1 has been delivered.

int main(...) {
    ...

    sigset_t ss_usr1;
    sigemptyset(&ss_usr1);
    sigaddset(&ss_usr1, SIGUSR1);

    // block SIGUSR1
    pthread_sigmask(SIG_BLOCK, &ss_usr1, NULL);

    ... spawn threads ...

    // RunServer
    struct timespec patience = { .tv_sec = 1 };

    while (! some_condition) {
        int s = sigtimedwait(&ss_usr1, NULL, &patience);
        if (s == SIGUSR1) dir_reinit();
        // handle errors other than timeout appropriately
    }
    server_exit();

    ...
}

Benefits: no signal handler complexity (and you won't be chastized, rightly, for using signal instead of the superior sigaction; no need for atomic flags or sig_atomic_t; easier to reason about behavior.

pilcrow
  • 56,591
  • 13
  • 94
  • 135