Bug#818478: monit: Leaves zombies around on timed out processes

[Guillem Jover]

Package: monit
Version: 1:5.17.1-1
Severity: normal
Tags: patch

Hi!

When a service is specified with a timeout, and that triggers, monit
leaves zombies around that it will never cleanup. This is rather
confusing because you are not sure if there's an actual problem with
the process itself, which in this case it's with the monitoring itself.

Here's a small reproducer:

,--- zombie.c ---
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int
main()
{
        pid_t pid;

        pid = fork();
        if (pid < 0) {
                fprintf(stderr, "error forking: %m\n");
                exit(2);
        } else if (pid == 0) {
                _exit(0);
        } else {
                FILE *fp = fopen("/var/run/zombie.pid", "w+");
                if (fp == NULL)
                        exit(1);
                fprintf(fp, "%d\n", pid);
                fclose(fp);

                sleep(120);
                exit(0);
        }

        return 0;
}
`---

,--- /etc/monit/monitrc.d/zombie ---
check process zombie with pidfile /var/run/zombie.pid
  start program = "/root/zombie" with timeout 2 seconds
  mode manual
`---

You should notice a normal zombie created by the zombie program, which
vanishes once monit kills the process, and the other one which sticks
around and never gets cleaned up. The problem is that monit does a
Process_free() on timeout, which kills the process but does not wait
for it, and subsequently loses track of the pid.

The attached patch fixes the issue for me.

Thanks,
Guillem

Description: When a process times out and we kill it, we should also wait
 for it, otherwise we leave zombies laying around, which are a sanitary
 hazard at best, and very confusing at worst.
Author: Guillem Jover <gjo...@sipwise.com>
Last-Update: 2016-03-17


--- monit-5.17.1.orig/libmonit/src/system/Command.c
+++ monit-5.17.1/libmonit/src/system/Command.c
@@ -233,12 +233,17 @@ static Process_T _Process_new(void) {
 
 
 void Process_free(Process_T *P) {
+        int isrunning;
+
         assert(P && *P);
         FREE((*P)->working_directory);
-        if (Process_isRunning(*P)) 
+        isrunning = Process_isRunning(*P);
+        if (isrunning)
                 Process_kill(*P);
         _closeParentPipes(*P);
         _closeStreams(*P);
+        if (isrunning)
+                Process_waitFor(*P);
         FREE(*P);
 }