Multithreaded Libevent/Libevent2 Server (Code Commentary)

I've been playing with libevent v2.x for a while. In the course of the "experiments", I come across Ron Cemer's  multithreaded libevent implementation (see http://roncemer.com/software-development/multi-threaded-libevent-server-example/ and http://sourceforge.net/projects/libevent-thread/) and also an updated version from Qi Huang (see http://randomindigits.blogspot.co.id/2012/11/libevent-2x-multithreaded-socket-server_4.html and https://github.com/gambellhq/samples/tree/master/libevent2-thread-code).

These multithreaded libevent code are mostly just fine except in one department, i.e. in signal handling. Both versions calls pthread functions from inside the signal handler (albeit not directly). The said practice is unsafe. It should have handle signals in synchronous pattern instead of asynchronous pattern. What I meant by this is the code should have used sigwait() in a dedicated thread to wait for signal(s) and call pthread functions from there instead of calling pthread function inside signal handler which is unsafe.

If you've been "system" programmer, you should already know that signal handler in *NIX runs in a different execution context compared to "normal" threads execution context. Therefore, you shouldn't call anything not prescribed beforehand outside of that "not-normal" execution context.

Now, let's see what exactly I mean (see echoserver_threaded.c and workqueue.c in Qi Huang code):

int runServer(void) {
    ...
    /* Set signal handlers */
    sigset_t sigset;
    sigemptyset(&sigset);
    struct sigaction siginfo = {
        .sa_handler = sighandler,
        .sa_mask = sigset,
        .sa_flags = SA_RESTART,
    };
    sigaction(SIGINT, &siginfo, NULL);
    sigaction(SIGTERM, &siginfo, NULL);
    ...
}
...
static void sighandler(int signal) {
    fprintf(stdout, "Received signal %d: %s.  Shutting down.\n", signal,
            strsignal(signal));
    killServer();
}
....
void killServer(void) {
    fprintf(stdout, "Stopping socket listener event loop.\n");
    if (event_base_loopexit(evbase_accept, NULL)) {
        perror("Error shutting down server");
    }
    fprintf(stdout, "Stopping workers.\n");
    workqueue_shutdown(&workqueue);
}
...
void workqueue_shutdown(workqueue_t *workqueue) {
    worker_t *worker = NULL;
    ...
    /* Remove all workers and jobs from the work queue.
     * wake up all workers so that they will terminate. */
    pthread_mutex_lock(&workqueue->jobs_mutex);
    ...
    pthread_cond_broadcast(&workqueue->jobs_cond);
    pthread_mutex_unlock(&workqueue->jobs_mutex);
}

As you can see in the excerpt above, the sighandler() function calls pthread functions indirectly. This practice is discouraged for sure because as per POSIX standard, pthread calls from signal handler is undefined. Therefore, it's unsafe to do so.

Anyway, feel free to challenge this view :-)

---

Post a Comment