1

Some time ago I wrote a simple SMTP gate for automatic S/MIME processing and now it comes to testing. As typical for mail servers, main process forks a child for every incoming connection. It is a good practice to limit the number of created child processes -- and so I done it.

During heavy load (many connections from many clients at the same time) it appears that child processes are not correctly counted -- the problem is in decreasing the counter when children exits. After a few minutes of heavy load counter is greater than actual number of child processes (ie. after 5 minutes it equals 14, but there are none).

I already did some research, but nothing worked. All zombie processes are reaped, so SIGCHLD handling seem to be ok. I thought that it may be a synchronization problem, but adding a mutex and changing variable type to volatile sig_atomic_t (as it is now) gives no change. It is also not a problem with signal masking, I tried masking all signal using sigfillset(&act.sa_mask).

I noticed that waitpid() sometimes returns strange PID values (very large, like 172915914).

Questions and some code.

  1. Is it possible that other process (ie. init) is reaping some of them?
  2. Can a process not become a zombie after exit? Can it be reaped automatically?
  3. How to fix it? Maybe there is a better way of counting them?

Forking a child in main():

volatile sig_atomic_t sproc_counter = 0;    /* forked subprocesses counter */

/* S/MIME Gate main function */
int main (int argc, char **argv)
{
    [...]

    /* set appropriate handler for SIGCHLD */
    Signal(SIGCHLD, sig_chld);

    [...]

    /* SMTP Server's main loop */
    for (;;) {

        [...]

        /* check whether subprocesses limit is not exceeded  */
        if (sproc_counter < MAXSUBPROC) {
            if ( (childpid = Fork()) == 0) {    /* child process */
                Close(listenfd);                /* close listening socket */
                smime_gate_service(connfd);     /* process the request */
                exit(0);
            }
            ++sproc_counter;
        }
        else
            err_msg("subprocesses limit exceeded, connection refused");

        [...]
    }
    Close(connfd);  /* parent closes connected socket */
}

Signal handling:

Sigfunc *signal (int signo, Sigfunc *func)
{
    struct sigaction    act, oact;

    act.sa_handler = func;
    sigemptyset(&act.sa_mask);
    act.sa_flags = 0;

    if (signo == SIGALRM) {
#ifdef  SA_INTERRUPT
        act.sa_flags |= SA_INTERRUPT;   /* SunOS 4.x */
#endif
    }
    else {
#ifdef  SA_RESTART
        act.sa_flags |= SA_RESTART;     /* SVR4, 44BSD */
#endif
    }
    if (sigaction(signo, &act, &oact) < 0)
        return SIG_ERR;

    return oact.sa_handler;
}

Sigfunc *Signal (int signo, Sigfunc *func)
{
    Sigfunc *sigfunc;

    if ( (sigfunc = signal(signo, func)) == SIG_ERR)
        err_sys("signal error");
    return sigfunc;
}

void sig_chld (int signo __attribute__((__unused__)))
{
    pid_t pid;
    int stat;

    while ( (pid = waitpid(-1, &stat, WNOHANG)) > 0) {
        --sproc_counter;
        err_msg("child %d terminated", pid);
    }
    return;
}

NOTE: All functions beginning with a capital letter (like Fork(), Close(), Signal() etc.) do and behaves the same as they lower case friends (fork(), close(), signal() etc.), but have better error handling -- so I don't have to check their return statuses.

NOTE2: I run and compile it under Debian Testing (kernel v3.10.11) using gcc 4.8.2.

3
  • Consider making your code call sig_chld() periodically, for example in a thread. Rather than in a signal handler function. Signal handlers like yours have the potential not to complete correctly when there is a barrage of signals. Which it seems is your problem. Jan 3, 2014 at 16:26
  • What does your Fork function do when fork fails?
    – Duck
    Jan 3, 2014 at 16:50
  • It prints an error message and exits calling exit(1).
    – TPhaster
    Jan 3, 2014 at 19:35

2 Answers 2

1

I think the signal method can be fixed, while creating a thread forces you to exec a program to handle a connection.

There are several problems:

  • Changes to sproc_counter may be lost if a process is created and ended at the same time. To fix this, either use signal masks (e.g., sigprocmask(), pselect()) to ensure the handler is not invoked while the main flow is manipulating sproc_counter, or make the signal handler set a flag and perform the waitpid(), counter manipulation and logging in the main flow (but not in a new thread). Note that the flag method still requires signal mask manipulation if you want to avoid sleeping for a new connection or another ending connection directly after an ending connection.

  • err_msg() is probably not async-signal safe. I see three options:

    • use the flag method mentioned above, or
    • ensure no async-signal unsafe functions are called while SIGCHLD is unmasked, or
    • remove the call from the signal handler.
  • Overriding signal() may cause other code to invoke your version instead of the standard version. This is likely to lead to strange behaviour.

  • The signal handler does not save and restore the value of errno.

If you have problems because of signals interrupting other signals, that's what sigaction's sa_mask field is for.

1
  • Yeah, the way with manipulating signal mask works well! And is much simpler. Thank you for this answer.
    – TPhaster
    Jan 14, 2014 at 2:50
0

I will answer myself.

There are several reasons for not to count child processes this way. Firstly signal handler could be interrupted by another signal. I can't find any information, what actually happens when this situation occurs. There are some info about it on libc manual pages and in this answer. But that may not be an issue.

It seems that operations on volatile sig_atomic_t variable are not really atomic and it depend on system architecture. In example, on amd64 compiled code of decrementing sproc_counter value looks like that:

movl    sproc_counter(%rip), %eax
subl    $1, %eax
movl    %eax, sproc_counter(%rip)

As you can see, there are as much as three assembler instructions! It's definitely not atomic, so access to sproc_counter have to be synchronized.

Okay, but why adding a mutex did not give a result? Answer is on manual page of pthread_mutex_lock()/pthread_mutex_unlock():

ASYNC-SIGNAL SAFETY

The mutex functions are not async-signal safe. What this means is that they should not be called from a signal handler. In particular, calling pthread_mutex_lock or pthread_mutex_unlock from a signal handler may deadlock the calling thread.

That makes it clear. What is more calling functions, which print date (log message) is also a bad idea -- used there fputs() is not async-signal-safe.

How to do it correctly?

Having in mind what can happen during signal handling (ie. delivery of other signals), it quite clear that signal handling routine should be as terse as possible. It is much better to set a flag in handler and test it from time to time in the main program or dedicated thread. I choose the second solution.

No more words, let's see some code.

Signal handling will look that way:

void sig_chld (int signo __attribute__((__unused__)))
{
  sigchld_notify = 1;
}

The main() routine:

volatile sig_atomic_t sigchld_notify = 0;                /* SIGCHLD notifier */
int sproc_counter = 0;                                   /* forked child process counter */
pthread_mutex_t sproc_mutex = PTHREAD_MUTEX_INITIALIZER; /* mutex for child process counter */

/* S/MIME Gate main function */
int main (int argc, char **argv)
{
    pthread_t guard_id;
    [...]

    /* start child process guard */
    if (0 != pthread_create(&guard_id, NULL, child_process_guard, NULL) )
        err_sys("pthread_create error");

    [...]

    /* SMTP Server's main loop */
    for (;;) {
        [...]

        /* check whether child processes limit is not exceeded */
        if (sproc_counter < MAXSUBPROC) {
            if ( (childpid = Fork()) == 0) { /* child process */
                Close(listenfd);             /* close listening socket */
                smime_gate_service(connfd);  /* process the request */
                exit(0);
            }
            pthread_mutex_lock(&sproc_mutex);
            ++sproc_counter;
            pthread_mutex_unlock(&sproc_mutex);
        }
        else
            err_msg("subprocesses limit exceeded, connection refused");

        Close(connfd); /* parent closes connected socket */
    }
} /* end of main() */

Guarding thread routine:

extern volatile sig_atomic_t sigchld_notify; /* SIGCHLD notifier */
extern int sproc_counter;                    /* forked child process counter */
extern pthread_mutex_t sproc_mutex;          /* mutex for child process counter */

void* child_process_guard (void* arg __attribute__((__unused__)))
{
    pid_t pid;
    int stat;

    for (;;) {
        if (0 == sigchld_notify) {
            usleep(SIGCHLD_SLEEP);
            continue;
        }

        while ( (pid = waitpid(-1, &stat, WNOHANG)) > 0) {
            pthread_mutex_lock(&sproc_mutex);
            --sproc_counter;
            pthread_mutex_unlock(&sproc_mutex);
            err_msg("child %d terminated", pid);
        }
        sigchld_notify = 0;
    }
    return NULL;
}
4
  • Note that smime_gate_service() may only use async-signal safe functions such as execve() if you do it this way. This is because fork() replicates only the calling thread in the new process, while other threads may own locks (including locks used internally by the system).
    – jilles
    Jan 10, 2014 at 22:32
  • Thanks for this comment, I did not come to think of how forking and threads interfere. But this is rather not an issue -- smime_gate_service() is a single-threaded, do not perform forks and don't have any signal handling. There are no more threads and forks than you see in this code excerpt. It's even better that only the main thread is replicated -- because that's the only one I need.
    – TPhaster
    Jan 11, 2014 at 12:44
  • There can still be a problem, for example if a fork happens while the child process guard thread is in err_msg(), and the child process also uses err_msg() later. The data structures may be inconsistent, or you may be waiting for a thread that does not exist in the child process to unlock something (in particular, stdio FILE objects are specified to be locked).
    – jilles
    Jan 11, 2014 at 17:57
  • Ah now I see, but that's kind of frightening vision -- and worse, it's real! Do you think enlarging critical sections is enough? (in main() to have fork() inside; in child_process_guard() to have err_msg() inside) Is waitpid() and usleep() safe for multithread forking?
    – TPhaster
    Jan 13, 2014 at 22:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.