I want the process exit when it receive an exit signal. So the main thread needs to wait for the exit signal, which I wrote like this before.

int signal(int a) {
    exit_flag = 0;
}
//wait in main thread
while(exit_flag){
    sleep(1);
}

But it reacts a little slowly. So I find a new method like this,

//signal function
int signal(int a) {
    pthread_mutex_unlock(&mutex);
}
//main thread

pthread_mutex_lock(&lock);
pthread_mutex_lock(&lock);

I'm not sure if it's correct. My question is that if it's a propper way to wait for an exit signal or you can tell me a better way?

---

Sincerely thank you all for your replies. I know that it's undefined beahiver and unsafe to use the mutex in signal handler. But is the above code correct if I send the exit message through some other method, like a jrpc call or something like that?

CodePudding user response：

My question is that if it's a propper way to wait for an exit signal or you can tell me a better way?

No, for reasons that @AndrewHenle describes very well in his answer.

There are multiple correct ways to do it, but many of them reinvent the wheel of the pause() function and its improved alternative sigsuspend(). These two are specifically for the purpose of waiting for a signal to be delivered. The Glibc manual contains a section on how to use them for the purpose.

Alternatively, if your process is multithreaded and you want to devote only one thread to waiting, then there is sigwait(), sigwaitinfo() and sigtimedwait().

For the whole process to wait

Suppose you want the whole process to stop until a SIGUSR1 is delivered to it, then exit. After installing your signal handler, you might use something like this:

// sufficient for this case even in a multi-threaded program:
volatile sig_atomic_t exit_flag;

// ...

/*
 * Wait, if necessary, until a SIGUSR1 is received, then exit with status 0.
 *
 * Assumes that a signal handler for SIGUSR1 is already installed, that the
 * handler will set variable `exit_flag` to nonzero when it runs, and that
 * nothing else will modify exit_flag incompatibly.
 */
void wait_to_exit(void) {
    sigset_t temp_mask, mask;

    sigemptyset(&temp_mask);
    sigaddset(&temp_mask, SIGUSR1);

    /*
     * Temporarily block the signal we plan to wait for, to ensure that we
     * don't miss a signal.
     */
    sigprocmask(SIG_BLOCK, &temp_mask, &mask);

    // Prepare to wait for the expected signal even if it is presently blocked
    sigdelset(&mask, SIGUSR1);

    // if we haven't already received the signal, then block the whole process until we do
    while (!exit_flag) {
        sigsuspend(&mask);
    }

    // No need to reset any signal masks because we're about to ...
    exit(0);
}

About the flag's data type

To expand on one of the comments in the above code, volatile sig_atomic_t is a sufficient type for the purpose even in a multithreaded program. sigsuspend() is specified to return after the signal handler returns, and the signal handler will not return until after the write to the flag actually happens (because of volatility). The thread calling sigsuspend must then read the value that was written by the handler, or some other value subsequently written to the same variable, again because of volatility.

volatile typically is not enough for thread safety, however, so even though it is not necessary in this case, you could consider sidestepping the issue and any uncertainty about it by instead using atomic_flag (declared in stdatomic.h); this requires support for C11 or later.

For just one thread to wait

For the case of one thread out of many waiting for a signal, it should be structured rather differently. You do not need a signal handler or flag in this case, but you should block the expected signal for all threads via sigprocmask():

    sigset_t mask;

    sigemptyset(&mask);
    sigaddset(&mask, SIGUSR1);

    sigprocmask(SIG_BLOCK, &mask, NULL);

That prevents the default (or custom) disposition of the expected signal from being executed in any thread, and ensures that threads other than the one that is to wait do not consume the signal. Typically, it should be done very early in the program's execution.

Then, to await the signal, a thread does this:

void wait_to_exit(void) {
    sigset_t mask;
    int sig;

    sigemptyset(&temp_mask);
    sigaddset(&temp_mask, SIGUSR1);

    // if we haven't already received the signal, then block this thread until we do
    if (sigwait(&mask, &sig) != 0) {
        // Something is terribly wrong
        fputs("sigwait failed\n", stderr);
        abort();
    }
    assert(sig == SIGUSR1);

    // Terminate the process (all threads)
    exit(0);
}

If you mean a generic "signal"

If you mean "signal" as a generic term for a synchronous notification, as opposed to an exercise of the C signal-handling facility, then @AndrewHenle's suggestion of a semaphore would be perfect. In that case, be sure to accept that answer.

CodePudding user response：

No, it is not correct.

First, pthread_mutex_unlock() is not an async-signal-safe function and can not be safely called from within a signal handler.

Second, mutexes are locked by a thread. If the signal handler is run in a different thread than the thread that has the mutex locked, it can not unlock the mutex:

If a thread attempts to unlock a mutex that it has not locked or a mutex which is unlocked, pthread_mutex_unlock() shall behave as described in the Unlock When Not Owner column of the following table.

The only entries in that table are "undefined behavior" and "error returned". And you have no real control over which thread the signal will be delivered to (at least not without writing complex signal-handling code...)

Third, this code

pthread_mutex_lock(&lock);
pthread_mutex_lock(&lock);

per that same table, will not block safely for any type of mutex. That code will either deadlock, continue with the mutex locked, or invoke undefined behavior which might even appear to "work" but leaves your program in an unknown state that could cause errors later.

Edit:

Fourth, if the signal is delivered multiple times, multiple calls to pthread_mutex_unlock() will again result either in errors or undefined behavior.

But there is an async-signal-safe way to block waiting for a signal: sem_wait().

sem_post() is async-signal-safe and can safely be called from within a signal handler, and can also be call multiple times safely - multiple calls to sem_post() will merely allow a corresponding number of calls to sem_wait() to obtain the semaphore, but you only need one to work:

//signal function
int signal(int a) {
    sem_post(&sem);
}
//main thread

sem_wait(&sem);

Note that it is not safe to call sem_wait() within a signal handler.

CodePudding user response：

Besides accepting signals (via sigwait or sigtimedwait), I'm a fan of the venerable self-pipe trick: "Maintain a pipe and select for readability on the pipe input. Inside the [signal] handler, write a byte (non-blocking, just in case) to the pipe output."

I'd further add to the above that the signal handler ought to be installed by sigaction as SA_RESTARTable.

Now you can safely mix the signal delivery with IO (via select or poll, e.g., or just blockingly read until that byte comes across).