You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by di...@covalent.net on 2002/05/13 23:45:12 UTC
Deamon tools
Hmm - not entirely trivial; it turns out that most unix-es do not take
kindly to
setsid()
when not detached/non-root. So I changed Jos/Michaels patch. See below.
Anyone any comments ?
Dw
Index: CHANGES
===================================================================
RCS file: /home/cvs/apache-1.3/src/CHANGES,v
retrieving revision 1.1817
diff -r1.1817 CHANGES
2a3,8
> *) Added a '-F' flag; which causes the mother/supervisor process to
> no longer fork down and detach. But instead stays attached to
> the tty - thus making live for automatic restart and exit checking
> code easier. [ Contributed by Michael Handler <ha...@grendel.net>,
> Jos Backus <jo...@catnook.com> [ Dirk-Willem van Gulik ]].
>
Index: main/http_main.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/http_main.c,v
retrieving revision 1.580
diff -r1.580 http_main.c
343a344,345
> static int do_detach = 1;
>
1352c1354
< fprintf(stderr, " %s [-v] [-V] [-h] [-l] [-L] [-S] [-t] [-T]\n", pad);
---
> fprintf(stderr, " %s [-v] [-V] [-h] [-l] [-L] [-S] [-t] [-T] [-F]\n", pad);
1360c1362
< fprintf(stderr, " %s [-v] [-V] [-h] [-l] [-L] [-S] [-t] [-T]\n", pad);
---
> fprintf(stderr, " %s [-v] [-V] [-h] [-l] [-L] [-S] [-t] [-T] [-F]\n", pad);
1382a1385
> fprintf(stderr, " -F : run main process in foreground, for process supervisors\n");
3377,3382c3380,3388
< if ((x = fork()) > 0)
< exit(0);
< else if (x == -1) {
< perror("fork");
< fprintf(stderr, "%s: unable to fork new process\n", ap_server_argv0);
< exit(1);
---
> if (do_detach) {
> if ((x = fork()) > 0)
> exit(0);
> else if (x == -1) {
> perror("fork");
> fprintf(stderr, "%s: unable to fork new process\n", ap_server_argv0);
> exit(1);
> }
> RAISE_SIGSTOP(DETACH);
3384d3389
< RAISE_SIGSTOP(DETACH);
3387c3392
< if ((pgrp = setsid()) == -1) {
---
> if ((do_detach) && ((pgrp = setsid()) == -1)) {
5315c5320
< "D:C:c:xXd:f:vVlLR:StTh"
---
> "D:C:c:xXd:Ff:vVlLR:StTh"
5336a5342,5344
> case 'F':
> do_detach = 0;
> break;
7218c7226
< while ((c = getopt(argc, argv, "D:C:c:Xd:f:vVlLz:Z:wiuStThk:n:W:")) != -1) {
---
> while ((c = getopt(argc, argv, "D:C:c:Xd:fF:vVlLz:Z:wiuStThk:n:W:")) != -1) {
7220c7228
< while ((c = getopt(argc, argv, "D:C:c:Xd:f:vVlLesStTh")) != -1) {
---
> while ((c = getopt(argc, argv, "D:C:c:Xd:fF:vVlLesStTh")) != -1) {
7341a7350,7352
> case 'F':
> do_detach = 0;
> break;
7736c7747
< while ((c = getopt(argc, argv, "D:C:c:Xd:f:vVlLR:SZ:tTh")) != -1) {
---
> while ((c = getopt(argc, argv, "D:C:c:Xd:Ff:vVlLR:SZ:tTh")) != -1) {
7742a7754
> case 'F':
--
Dirk-Willem van Gulik
Re: Deamon tools
Posted by Aaron Bannert <aa...@clove.org>.
> Note that I'm not sure if httpd 2.0 is this robust in handling
> setsid(2), so if we're spending the time on this and it's considered
> important for 1.3, someone should probably spend some time on making
> 2.0 as robust.
I improved the error detecting in 2.0 when originally adding Jos' patch
there, and apr_proc_detach returns an error if setsid() or setpgrp()
fails internally, but the error message may still be lacking (I wasn't
sure if we wanted to assume that a failed apr_proc_detach() was because
of a -DNO_DETACH+no-daemontools situation or not).
-aaron
Re: Deamon tools
Posted by Michael Handler <ha...@grendel.net>.
Aaron Bannert <aa...@clove.org> writes:
> I think we should just fail with an error if the user wished to start
> up under daemontools mode (no_detach mode) but somehow started httpd
> as a process group leader.
Concur.
> On Mon, May 13, 2002 at 03:15:44PM -0700, dirkx@covalent.net wrote:
> > In that case I'd suggest we do
> >
> > if (setsid() fails)
> > always log error
> > exit(1) unless no_detach.
>
> should that be
> exit(1) if no_detach
> ? or do I have the logic backward?
That's correct, dirk had the test inverted.
Note that I'm not sure if httpd 2.0 is this robust in handling
setsid(2), so if we're spending the time on this and it's considered
important for 1.3, someone should probably spend some time on making
2.0 as robust.
--michael
Re: Deamon tools
Posted by di...@covalent.net.
> What do you mean? At what point do we stop trying to detect errors
> and let the daemon go on its merry way?
Ideally once she says:
[Mon May 13 14:39:24 2002] [notice] Apache/1.3.25-dev (Unix) configured -- resuming normal operations
which if I recall correctly is the moment after which you can only get
errors/permission faults which are to be triggered by request/runtime sort
of things (unless the first flock() is a bit later).
Dw
Re: Deamon tools
Posted by Aaron Bannert <aa...@clove.org>.
> Of course the real patch is to make the exit codes always reliable - even
> when we fork and detach ;-)
What do you mean? At what point do we stop trying to detect errors
and let the daemon go on its merry way?
-aaron
Re: Deamon tools
Posted by di...@covalent.net.
> As long as you still do the fork(2) decision before the setsid(2) call,
> no. :)
Of course the real patch is to make the exit codes always reliable - even
when we fork and detach ;-)
Dw
Re: Deamon tools
Posted by Michael Handler <ha...@grendel.net>.
Aaron Bannert <aa...@clove.org> writes:
> Now that I think about it a little more, we should probably always
> exit if setsid() fails.
That sounds good. The only instance I can see setsid(2) legitimately
failing other than OS resource exhaustion or something is when you
invoke httpd -F interactively under a shell with job control, and
well, if you need to do that, invoke a non-job control shell first.
> It might be useful to change the error message
> if no_detach is set ("setsid() failed probably because you aren't
> running under a process management tool like daemontools" vs "setsid()
> failed, unable to create new session"). Then we just do the fork fork()
> if !no_detach. Confused yet?
As long as you still do the fork(2) decision before the setsid(2) call,
no. :)
Re: Deamon tools
Posted by Aaron Bannert <aa...@clove.org>.
> > > if (setsid() fails)
> > > always log error
> > > exit(1) unless no_detach.
> >
> > should that be
> > exit(1) if no_detach
> > ? or do I have the logic backward?
>
> We should exit if setsid() fails and we don't want to detach (i.e.
> no_detach). If we don't exit at that point we will end up killing processes in
> the wrong pgrp later. So I think Aaron's right.
Now that I think about it a little more, we should probably always
exit if setsid() fails. It might be useful to change the error message
if no_detach is set ("setsid() failed probably because you aren't
running under a process management tool like daemontools" vs "setsid()
failed, unable to create new session"). Then we just do the fork fork()
if !no_detach. Confused yet?
-aaron
Re: Deamon tools
Posted by di...@covalent.net.
> the wrong pgrp later. So I think Aaron's right.
Ok - committed - please check that it matches people their consensus :-).
Dw
Re: Deamon tools
Posted by Jos Backus <jo...@catnook.com>.
On Mon, May 13, 2002 at 03:23:54PM -0700, Aaron Bannert wrote:
> On Mon, May 13, 2002 at 03:15:44PM -0700, dirkx@covalent.net wrote:
> > In that case I'd suggest we do
> >
> > if (setsid() fails)
> > always log error
> > exit(1) unless no_detach.
>
> should that be
> exit(1) if no_detach
> ? or do I have the logic backward?
We should exit if setsid() fails and we don't want to detach (i.e.
no_detach). If we don't exit at that point we will end up killing processes in
the wrong pgrp later. So I think Aaron's right.
--
Jos Backus _/ _/_/_/ Santa Clara, CA
_/ _/ _/
_/ _/_/_/
_/ _/ _/ _/
jos@catnook.com _/_/ _/_/_/ use Std::Disclaimer;
Re: Deamon tools
Posted by di...@covalent.net.
On Mon, 13 May 2002, Aaron Bannert wrote:
> On Mon, May 13, 2002 at 03:15:44PM -0700, dirkx@covalent.net wrote:
> > In that case I'd suggest we do
> >
> > if (setsid() fails)
> > always log error
> > exit(1) unless no_detach.
>
> should that be
> exit(1) if no_detach
> ? or do I have the logic backward?
Yes - sorry.
Re: Deamon tools
Posted by Aaron Bannert <aa...@clove.org>.
On Mon, May 13, 2002 at 03:15:44PM -0700, dirkx@covalent.net wrote:
> In that case I'd suggest we do
>
> if (setsid() fails)
> always log error
> exit(1) unless no_detach.
should that be
exit(1) if no_detach
? or do I have the logic backward?
-aaron
Re: Deamon tools
Posted by Jos Backus <jo...@catnook.com>.
On Mon, May 13, 2002 at 03:04:01PM -0700, Aaron Bannert wrote:
> I think we should just fail with an error if the user wished to start
> up under daemontools mode (no_detach mode) but somehow started httpd
> as a process group leader.
That's a good idea ("Doctor, it hurts when I poke here." ...).
> -aaron
--
Jos Backus _/ _/_/_/ Santa Clara, CA
_/ _/ _/
_/ _/_/_/
_/ _/ _/ _/
jos@catnook.com _/_/ _/_/_/ use Std::Disclaimer;
Re: Deamon tools
Posted by di...@covalent.net.
> OTOH, simply calling httpd from the command line (or exec'ing it from
> a shell script) will usually mean that the process is the leader of a
> new process group, meaning setsid() will fail.
I had not considered that mode of use - should we allow for that ? It
would propably make sense - seems useful to me.
In that case I'd suggest we do
if (setsid() fails)
always log error
exit(1) unless no_detach.
Dw
Re: Deamon tools
Posted by Aaron Bannert <aa...@clove.org>.
On Mon, May 13, 2002 at 02:45:12PM -0700, dirkx@covalent.net wrote:
>
> Hmm - not entirely trivial; it turns out that most unix-es do not take
> kindly to
>
> setsid()
>
> when not detached/non-root. So I changed Jos/Michaels patch. See below.
> Anyone any comments ?
Which platforms disallow non-root calls to setsid()? I thought the only
requirement for calling setsid() was that the calling process wasn't
already the leader of a process group. Daemontools ensurs this by forking
and exec()ing the process it is going to manage, meaning that if the
parent was a process group leader than the child is guaranteed to not be.
OTOH, simply calling httpd from the command line (or exec'ing it from
a shell script) will usually mean that the process is the leader of a
new process group, meaning setsid() will fail.
I think we should just fail with an error if the user wished to start
up under daemontools mode (no_detach mode) but somehow started httpd
as a process group leader.
-aaron
Re: Deamon tools
Posted by Jos Backus <jo...@catnook.com>.
On Mon, May 13, 2002 at 02:50:34PM -0700, dirkx@covalent.net wrote:
> > Hmm - not entirely trivial; it turns out that most unix-es do not take
> > kindly to
> >
> > setsid()
> >
> > when not detached/non-root. So I changed Jos/Michaels patch. See below.
> > Anyone any comments ?
>
> The other obvious way is to make the error non fatal or do a (!geteuid())
> to see if we are running as root.
But wait, if we skip the setsid() if !do_detach we will end up killing the
parent pgrp which contains - in this case - svscan/supervise/others, which is
part of the point of this patch. Am I missing something?
--
Jos Backus _/ _/_/_/ Santa Clara, CA
_/ _/ _/
_/ _/_/_/
_/ _/ _/ _/
jos@catnook.com _/_/ _/_/_/ use Std::Disclaimer;
Re: Deamon tools
Posted by di...@covalent.net.
> Hmm - not entirely trivial; it turns out that most unix-es do not take
> kindly to
>
> setsid()
>
> when not detached/non-root. So I changed Jos/Michaels patch. See below.
> Anyone any comments ?
The other obvious way is to make the error non fatal or do a (!geteuid())
to see if we are running as root.
Dw