Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Race condition in C Signal handler

Tags:

c

linux

signals

I'm doing some coursework, and we've been presented with the following code. Some of the questions ask what various lines of the code do, which is fine, and I understand, but the curveball is "This program contains a race condition. Where and why does it arise?"

The code:

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

static void handler(int signo) { 
    printf("This is the SIGUSR1 signal handler!\n");
}

int main(void) { 
    sigset_t set;
    sigemptyset(&set);
    sigset(SIGUSR1, handler);
    sigprocmask(SIG_SETMASK, &set, NULL);

    while(1) {
        printf("This is main()!\n");
    }
    return 0;

}

I'm thinking along the lines that the race condition is that there is no way to know what order "This is main" or "This is the SIGUSR1" will be printed when the signal arrives, but if anyone could confirm or clarify this I'd much appreciate it. He also asks how it could be fixed (the race condition), not looking for a full answer but any tips would be appreciated.

like image 980
Saf Avatar asked Dec 15 '22 20:12

Saf


1 Answers

Apparently the intent in the course is to modify the code to

  • Use a separate thread, which receives the signals calling sigwait() or sigwaitinfo() in a loop. The signals must be blocked (first, and all along, for all threads), or the operation is unspecified (or the signals are delivered to the other thread).

    This way there is no signal handler function per se, which would be limited to async-signal safe functions. The thread that calls sigwait()/sigwaitinfo() is a perfectly normal thread, and not limited in any way related to signals or signal handlers.

    (There are other ways to receive the signals, for example using a signal handler which sets a global flag, and which the loop checks. Most of those lead to busy-waiting, running a do-nothing loop, burning CPU time uselessly: a very bad solution. The way I describe here squanders no CPU time: the kernel will put the thread to sleep when it calls sigwait()/sigwaitinfo(), and wakes it up only when a signal arrives. If you want to limit the duration of the sleep, you can use sigtimedwait() instead.)

  • Since printf() et al. are not guaranteed to be thread-safe, you should probably use a pthread_mutex_t to protect output to standard output -- in other words, so that the two threads won't try to output at exactly the same time.

    In Linux this is not necessary, since GNU C printf() (except for the _unlocked() versions) is thread-safe; each call to these functions uses an internal mutex already.

    Note that the C library may cache outputs, so to make sure the data is output, you need to call fflush(stdout);.

    The mutex is especially useful, if you want to use multiple printf(), fputs(), or similar calls atomically, without the other thread being able to inject output in between. Therefore, the mutex is recommended even if on Linux in simple cases it is not required. (And yes, you do want to do the fflush() too while holding the mutex, although it may cause the mutex to be held for a long time if the output blocks.)

I'd personally solve the entire issue completely differently -- I'd use write(STDERR_FILENO,) in the signal handler to output to standard error, and have the main program output to standard output; no threads or anything special needed, just a simple low-level write loop in the signal handler. Strictly speaking, my program would behave differently, but to the end user the results look very much the same. (Except that one could redirect the outputs to different terminal windows, and look at them side-by-side; or redirect them to helper scripts/programs which prepend nanosecond wall-clock timestamps to each input line; and other similar tricks useful when investigating things.)

Personally, I found the jump from the original problem to the "correct solution" -- if indeed what I am describing is the correct solution; I do believe it is -- to be a bit of a stretch. This approach only dawned on me when Saf mentioned that the correct solution is expected to use pthreads.

I hope you find this informative, but not a spoiler.


Edited 2013-03-13:

Here is the writefd() function I use to safely write data from a signal handler to a descriptor. I also included the wrapper functions wrout() and wrerr() that you can use to write strings to standard output or standard error, respectively.

#include <unistd.h>
#include <string.h>
#include <errno.h>

/**
 * writefd() - A variant of write(2)
 *
 * This function returns 0 if the write was successful, and the nonzero
 * errno code otherwise, with errno itself kept unchanged.
 * This function is safe to use in a signal handler;
 * it is async-signal-safe, and keeps errno unchanged.
 *
 * Interrupts due to signal delivery are ignored.
 * This function does work with non-blocking sockets,
 * but it does a very inefficient busy-wait loop to do so.
*/
int writefd(const int descriptor, const void *const data, const size_t size)
{
    const char       *head = (const char *)data;
    const char *const tail = (const char *)data + size;
    ssize_t           bytes;
    int               saved_errno, retval;

    /* File descriptor -1 is always invalid. */
    if (descriptor == -1)
        return EINVAL;

    /* If there is nothing to write, return immediately. */
    if (size == 0)
        return 0;

    /* Save errno, so that it can be restored later on.
     * errno is a thread-local variable, meaning its value is
     * local to each thread, and is accessible only from the same thread.
     * If this function is called in an interrupt handler, this stores
     * the value of errno for the thread that was interrupted by the
     * signal delivery. If we restore the value before returning from
     * this function, all changes this function may do to errno
     * will be undetectable outside this function, due to thread-locality.
    */
    saved_errno = errno;

    while (head < tail) {

        bytes = write(descriptor, head, (size_t)(tail - head));

        if (bytes > (ssize_t)0) {
            head += bytes;

        } else
        if (bytes != (ssize_t)-1) {
            errno = saved_errno;
            return EIO;

        } else
        if (errno != EINTR && errno != EAGAIN && errno != EWOULDBLOCK) {
            /* EINTR, EAGAIN and EWOULDBLOCK cause the write to be
             * immediately retried. Everything else is an error. */
            retval = errno;
            errno = saved_errno;
            return retval;
        }
    }

    errno = saved_errno;
    return 0;
}

/**
 * wrout() - An async-signal-safe alternative to fputs(string, stdout)
 *
 * This function will write the specified string to standard output,
 * and return 0 if successful, or a nonzero errno error code otherwise.
 * errno itself is kept unchanged.
 *
 * You should not mix output to stdout and this function,
 * unless stdout is set to unbuffered.
 *
 * Unless standard output is a pipe and the string is at most PIPE_BUF
 * bytes long (PIPE_BUF >= 512), the write is not atomic.
 * This means that if you use this function in a signal handler,
 * or in multiple threads, the writes may be interspersed with each other.
*/
int wrout(const char *const string)
{
    if (string)
        return writefd(STDOUT_FILENO, string, strlen(string));
    else
        return 0;
}

/**
 * wrerr() - An async-signal-safe alternative to fputs(string, stderr)
 *
 * This function will write the specified string to standard error,
 * and return 0 if successful, or a nonzero errno error code otherwise.
 * errno itself is kept unchanged.
 *
 * You should not mix output to stderr and this function,
 * unless stderr is set to unbuffered.
 *
 * Unless standard error is a pipe and the string is at most PIPE_BUF
 * bytes long (PIPE_BUF >= 512), the write is not atomic.
 * This means that if you use this function in a signal handler,
 * or in multiple threads, the writes may be interspersed with each other.
*/
int wrerr(const char *const string)
{
    if (string)
        return writefd(STDERR_FILENO, string, strlen(string));
    else
        return 0;
}

If the file descriptor refers to a pipe, writefd() can be used to write up to PIPE_BUF (at least 512) bytes atomically. writefd() can also be used in an I/O intensive application to convert signals (and if raised using sigqueue(), the related value, an integer or a pointer) to a socket or pipe output (data), making it much easier to multiplex multiple I/O streams and signal handling. A variant (with an extra file descriptor marked close-on-exec) is often used to easily detect whether a child process executed another process, or failed; otherwise it is difficult to detect which process -- the original child, or the executed process -- exited.

In the comments to this answer, there was some discussion about errno, and confusion whether the fact that write(2) modifies errno makes it unsuitable for signal handlers.

First, POSIX.1-2008 (and earlier) defines async-signal-safe functions as those that can safely be called from signal handlers. The 2.4.3 Signal actions chapter includes a list of such functions, including write(). Note that it also explicitly states that "Operations which obtain the value of errno and operations which assign a value to errno shall be async-signal-safe."

That means that POSIX.1 intends write() to be safe to use in a signal handler, and that errno can also be manipulated to avoid the interrupted thread seeing unexpected changes in errno.

Because errno is a thread-local variable, each thread has their own errno. When a signal is delivered, it always interrupts one of the existing threads in the process. Signals can be directed to a specific thread, but usually the kernel decides which thread gets a process-wide signal; it varies between systems. If there is only one thread, the initial or main thread, then obviously it is the one that gets interrupted. All this means that if the signal handler saves the value of errno it initially sees, and restores it just before it returns, the changes to errno are invisible outside the signal handler.

There is one way to detect it, though, also hinted at in POSIX.1-2008 by the careful wording:

Technically, &errno is almost always valid (depending on system, compiler, and standards applied), and yields the address of the int variable holding the error code for the current thread. It is therefore possible for another thread to monitor the error code of another thread, and yes, this thread would see the changes to it done within the signal handler. However, there is no guarantee that the other thread is able to access the error code atomically (although it is atomic on many architectures): such "monitoring" would be informative only anyway.

It is a pity that almost all signal handler examples in C use stdio.h printf() et cetera. Not only is it wrong on many levels -- from non-async-safe to the caching issues, possibly non-atomic accesses to FILE fields, if the interrupted code was also doing I/O at the same time --, but the correct solution using unistd.h similar to my example in this edit is just as simple. The rationale for using stdio.h I/O in signal handlers seems to be "it usually works". I detest that, personally, since for example violence also "usually works". I consider it stupid and/or lazy.

I hope you found this informative.

like image 89
Nominal Animal Avatar answered Jan 02 '23 05:01

Nominal Animal