[Cc'd the FreeBSD Apache2 port committer because he may be interested in this
discussion.]
Hi Justin,
On Thu, Dec 27, 2001 at 10:07:31PM -0800, Justin Erenkrantz wrote:
> On Thu, Dec 27, 2001 at 05:44:22PM -0800, Jos Backus wrote:
> > I'm willing to code a patch which allows Apache to run in the foreground in
> > its own session. Currently it kills the pgrp it is in even though it didn't
> > create it (bad practice imo - only destroy what you create).
>
> I'm not sure what version you are talking about (1.3 or 2.0), but
> on 2.0 Unix-based MPMs, you can run httpd with -DNO_DETACH and it
> will not detach from the terminal.
>
> ./httpd -DNO_DETACH
>
> The only place I'm seeing us killing the process group is when you
> send a SIGTERM. Is this what you are talking about?
Yes, this is with 2.0.28 (the version in the FreeBSD Ports collection) and it
happens with -DNO_DETACH; yes, we call setpgrp()/setsid()/etc. in
apr_proc_detach() but that function is not called when using -DNO_DETACH :-)
What would work in this particular case is to continue to call
apr_proc_detach() but telling it not to call fork(). Something along these
lines:
lizzy:/usr/ports/www/apache2/work/httpd-2_0_28# diff -u ./srclib/apr/threadproc/
unix/procsup.c.orig ./srclib/apr/threadproc/unix/procsup.c
--- ./srclib/apr/threadproc/unix/procsup.c.orig Fri Dec 28 00:39:58 2001
+++ ./srclib/apr/threadproc/unix/procsup.c Fri Dec 28 00:40:59 2001
@@ -54,7 +54,7 @@
#include "threadproc.h"
-APR_DECLARE(apr_status_t) apr_proc_detach(void)
+APR_DECLARE(apr_status_t) apr_proc_detach(int dont_fork)
{
int x;
pid_t pgrp;
@@ -63,13 +63,14 @@
#if !defined(MPE) && !defined(OS2) && !defined(TPF) && !defined(BEOS)
/* Don't detach for MPE because child processes can't survive the death of
the parent. */
- if ((x = fork()) > 0)
- exit(0);
- else if (x == -1) {
- perror("fork");
- fprintf(stderr, "unable to fork new process\n");
- exit(1); /* we can't do anything here, so just exit. */
- }
+ if (!dont_fork)
+ if ((x = fork()) > 0)
+ exit(0);
+ else if (x == -1) {
+ perror("fork");
+ fprintf(stderr, "unable to fork new process\n");
+ exit(1); /* we can't do anything here, so just exit. */
+ }
/* RAISE_SIGSTOP(DETACH);*/
#endif
#if APR_HAVE_SETSID
Of course, all the callers of apr_proc_detach() would need to be modified
(easy) and a new -D option would need to be implemented which causes this
behavior. This would make Apache play nice with AIX's System Resource
Controller as well as Dan Bernstein's daemontools. As a sysadmin, this is
something I am interested in. I am of course willing to do the rest of the
coding and doc updates for this.
Btw, there is a macro bug affecting apr_proc_detach(): it checks for
APR_HAVE_SETSID but configure sets HAVE_SETSID. One FreeBSD (and other
platforms that have setsid()) this causes this code to be used instead of
setsid():
if ((pgrp = setpgid(0, 0)) == -1) {
return errno;
}
FreeBSD's setpgid(2) says
Setpgid() sets the process group of the specified process pid to the
specified pgrp. If pid is zero, then the call applies to the current
process.
So the use of the second 0 strikes me as suspicious and likely wrong. The
FreeBSD Apache port currently uses a patch located at
http://fallin.lv/distfiles/apache-2.0.28_1.diff; maybe you can verify this and
commit this fix so the patch can go away. Hye-Shik Chang told me he entered a
Apache problem report for this issue.
Btw, I reported this problem earlier in a mail to apache-dev (Subject:
HAVE_SETSID problem, date: Thu, 27 Dec 2001 17:44:22 -0800)
> Do you have a better mechanism in mind?
See above, hope that explains things well enough.
> AIUI, some OSes do not deliver signals
> sent to the parent to the children and others do. I think we were
> attempting to obtain consistency. I imagine we could have httpd
> set it own process group, but that might not be good either.
> Thoughts? -- justin
As I see it, it would be sufficient to do the setsid() without the fork().
Fwiw, I just sent off a very similar patch to the Samba people in response to
an e-mail response by Jeremy Allison; nmbd and smbd have the same problem and
the fix is very similar.
Thank you!
--
Jos Backus _/ _/_/_/ Santa Clara, CA
_/ _/ _/
_/ _/_/_/
_/ _/ _/ _/
[EMAIL PROTECTED] _/_/ _/_/_/ use Std::Disclaimer;
