dovecot - Jan 2011 - Dovecot 2.0.7 doesn't disassociate STDERR when it daemonizes.

At least it appears that way on my FreeBSD 8.1 system.  This is probably
unnoticeable in regular use but I happen to use CFengine(v2) for system
configuration & monitoring.  If it notices dovecot not running it tries
to start it and hangs.

I believe cfengine (cfagent) has created a pipe to monitor the start
commmand's stderr and is waiting for this pipe to close.  This never
happens because the pipe has been inherited by dovecot which, in version
2, doesn't release stderr as part of its daemonization. Although
cfengine could be more defensive, I believe dovecot should be
disassociating from the inherited stderr.

Looking at master/main.c, I see stdin, stdout & stderr being set to
/dev/null in main() if they are inherited unassigned:

        do {
                null_fd = open("/dev/null", O_WRONLY);
                if (null_fd == -1)
                        i_fatal("Can't open /dev/null: %m");
                                       fd_close_on_exec(null_fd, TRUE);
        } while (null_fd <= STDERR_FILENO);

But stderr is missing when a later dup2() is used to release any
inherited values not covered by the above:

       if (dup2(null_fd, STDIN_FILENO) < 0 ||
            dup2(null_fd, STDOUT_FILENO) < 0)
                i_fatal("dup2(null_fd) failed: %m");

Adding a dup2 for stderr to the above statement works in my simple test
case but I'm unsure if there are other issues which may require delaying
it.  Dovecot 1.2.16 approaches this a little differently & does slightly
delay the stderr dup2.  The appended diff (v2.0.7) works for me but a
more informed opinion might have a better suggestion.

Thank you very much for your time.  I just started using Dovecot and really
like it.  -virgil

--- main.c.orig 2010-11-04 11:58:48.000000000 -0700
+++ main.c      2011-01-05 18:11:45.000000000 -0800
@@ -717,7 +717,8 @@
        }

        if (dup2(null_fd, STDIN_FILENO) < 0 ||
-           dup2(null_fd, STDOUT_FILENO) < 0)
+           dup2(null_fd, STDOUT_FILENO) < 0 ||
+           dup2(null_fd, STDERR_FILENO) < 0)
                i_fatal("dup2(null_fd) failed: %m");

        pidfile_path 
-=-=-=-=-=-=-=-=-=-=-

On Thu, 2011-01-06 at 10:42 -0800, Virgil Champlin
wrote:
> --- main.c.orig 2010-11-04 11:58:48.000000000 -0700
> +++ main.c      2011-01-05 18:11:45.000000000 -0800
> @@ -717,7 +717,8 @@
>         }
> 
>         if (dup2(null_fd, STDIN_FILENO) < 0 ||
> -           dup2(null_fd, STDOUT_FILENO) < 0)
> +           dup2(null_fd, STDOUT_FILENO) < 0 ||
> +           dup2(null_fd, STDERR_FILENO) < 0)
>                 i_fatal("dup2(null_fd) failed: %m");
> 
This is done a bit too early.
http://hg.dovecot.org/dovecot-2.0/rev/08e4280e5bfd probably fixes it
too?