You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Dean Gaudet <dg...@arctic.org> on 1997/11/09 12:52:38 UTC
[PATCH] pool confusion (take 1)
The current code has two related bugs:
- flock doesn't seem to be cleaning up its file on
SIGTERM under Linux
- the register_other_child API is getting "unregister"
events when it shouldn't (this one is on the STATUS already)
They're related because what's happened is that all httpd children are
calling destroy_pool(pconf). This is wrong.
Alexei added this when he was fixing up the child_exit API in August, you
can read that thread if you want to know what he did... but he made the
children start calling destroy_pool(pconf) claiming it was the right thing
to do. I'm now claiming it's not the right thing to do.
There are three cleanups that http_main has to perform:
- cleanup at the end of a request
- cleanup at the end of life of a child
- cleanup at USR1/HUP/TERM of the parent
We've overloaded pconf with the last two of those. This just won't work;
there are cleanups that need to be done exactly once (i.e. unlinking
the scoreboard, or the flock file, or deleting the sysvsem). Right now
we hack around that by listing a bunch of crud at the exit() point of
the parent. This is silly, we have the concept of a cleanup built into
pools, we should be able to use it.
This patch creates "pool *pchild;", which has the second meaning above.
"pconf" is cleaned up only by the parent. It removes various cleanup
definitions in the mutex and scoreboard code, and replaces them with
register_cleanup() calls as appropriate (which simplifies other code).
It attempts to destroy_pool(pchild) or destroy_pool(pconf) at the
appropriate moments.
Tested so far:
- Linux with flock, with fcntl, with sysvsem; SIGUSR1, HUP, TERM
- Linux with shmget, and with scoreboard files; SIGUSR1, HUP, TERM
Remaining to do:
- test Solaris with pthread, mmap
- test IRIX with uslock if I get motivated enough
- figure out what other signals we can trap and rationally clean up on;
this is important -- we can clean up on a lot of other signals,
like SIGXCPU, and SIGFPE. We should do that cleanup because we
have a few configurations in which cleanups are important.
(i.e. pthreads, flock, sysvsem, qnx mmap, file-based scoreboard)
- win32 -- I figure someone else can do this after commit, it already has
a pchild that I think has the same meaning
Note: The child_exit API is redundant once this patch is in place,
because any module desiring a cleanup at child exit can simply register
a pool cleanup on pchild during the child_init function.
Thoughts? Complaints?
Dean
Index: alloc.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/alloc.c,v
retrieving revision 1.57
diff -u -r1.57 alloc.c
--- alloc.c 1997/11/06 21:54:07 1.57
+++ alloc.c 1997/11/09 11:34:06
@@ -936,6 +936,11 @@
#endif /* ndef WIN32 */
}
+API_EXPORT(void) null_cleanup(void *data)
+{
+ /* do nothing cleanup routine */
+}
+
/*****************************************************************
*
* Files and file descriptors; these are just an application of the
Index: alloc.h
===================================================================
RCS file: /export/home/cvs/apachen/src/main/alloc.h,v
retrieving revision 1.36
diff -u -r1.36 alloc.h
--- alloc.h 1997/10/26 21:14:16 1.36
+++ alloc.h 1997/11/09 11:34:06
@@ -194,6 +194,10 @@
API_EXPORT(void) kill_cleanup(pool *p, void *data, void (*plain_cleanup) (void *));
API_EXPORT(void) run_cleanup(pool *p, void *data, void (*cleanup) (void *));
+/* A "do-nothing" cleanup, for register_cleanup; it's faster to do
+ * things this way than to test for NULL. */
+API_EXPORT(void) null_cleanup(void *data);
+
/* The time between when a resource is actually allocated, and when it
* its cleanup is registered is a critical section, during which the
* resource could leak if we got interrupted or timed out. So, anything
Index: http_main.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_main.c,v
retrieving revision 1.247
diff -u -r1.247 http_main.c
--- http_main.c 1997/11/08 19:19:15 1.247
+++ http_main.c 1997/11/09 11:34:07
@@ -242,6 +242,7 @@
pool *pconf; /* Pool for config stuff */
pool *ptrans; /* Pool for per-transaction stuff */
+pool *pchild; /* Pool for httpd child stuff */
int APACHE_TLS my_pid; /* it seems silly to call getpid all the time */
#ifndef MULTITHREAD
@@ -278,8 +279,6 @@
static ulock_t uslock = NULL;
-#define accept_mutex_cleanup()
-
#define accept_mutex_child_init(x)
static void accept_mutex_init(pool *p)
@@ -351,7 +350,7 @@
static sigset_t accept_block_mask;
static sigset_t accept_previous_mask;
-static void accept_mutex_child_cleanup(void *data)
+static void accept_mutex_child_cleanup(void *foo)
{
if (accept_mutex != (void *)(caddr_t)-1
&& have_accept_mutex) {
@@ -359,7 +358,12 @@
}
}
-static void accept_mutex_cleanup(void)
+static void accept_mutex_child_init(pool *p)
+{
+ register_cleanup(p, NULL, accept_mutex_child_cleanup, null_cleanup);
+}
+
+static void accept_mutex_cleanup(void *foo)
{
if (accept_mutex != (void *)(caddr_t)-1
&& munmap((caddr_t) accept_mutex, sizeof(*accept_mutex))) {
@@ -368,8 +372,6 @@
accept_mutex = (void *)(caddr_t)-1;
}
-#define accept_mutex_child_init(x)
-
static void accept_mutex_init(pool *p)
{
pthread_mutexattr_t mattr;
@@ -403,8 +405,7 @@
sigdelset(&accept_block_mask, SIGHUP);
sigdelset(&accept_block_mask, SIGTERM);
sigdelset(&accept_block_mask, SIGUSR1);
- register_cleanup(pconf, NULL, accept_mutex_child_cleanup,
- accept_mutex_child_cleanup);
+ register_cleanup(p, accept_mutex_cleanup, null_cleanup);
}
static void accept_mutex_on()
@@ -462,26 +463,20 @@
#endif
-static int sem_cleanup_registered;
-static pid_t sem_cleanup_pid;
static int sem_id = -1;
static struct sembuf op_on;
static struct sembuf op_off;
/* We get a random semaphore ... the lame sysv semaphore interface
* means we have to be sure to clean this up or else we'll leak
- * semaphores. Note that this is registered via atexit, so we can't
- * do many things in here... especially nothing that would allocate
- * memory or use a FILE *.
+ * semaphores.
*/
-static void accept_mutex_cleanup(void)
+static void accept_mutex_cleanup(void *foo)
{
union semun ick;
if (sem_id < 0)
return;
- if (getpid() != sem_cleanup_pid)
- return;
/* this is ignored anyhow */
ick.val = 0;
semctl(sem_id, 0, IPC_RMID, ick);
@@ -494,15 +489,6 @@
union semun ick;
struct semid_ds buf;
- if (!sem_cleanup_registered) {
- /* only the parent will try to do cleanup */
- sem_cleanup_pid = getpid();
- if (atexit(accept_mutex_cleanup)) {
- perror("atexit");
- exit(1);
- }
- sem_cleanup_registered = 1;
- }
/* acquire the semaphore */
sem_id = semget(IPC_PRIVATE, 1, IPC_CREAT | 0600);
if (sem_id < 0) {
@@ -527,6 +513,7 @@
exit(1);
}
}
+ register_cleanup(p, NULL, accept_mutex_cleanup, null_cleanup);
/* pre-initialize these */
op_on.sem_num = 0;
@@ -559,8 +546,6 @@
static int lock_fd = -1;
-#define accept_mutex_cleanup()
-
#define accept_mutex_child_init(x)
/*
@@ -618,7 +603,7 @@
static int lock_fd = -1;
-static void accept_mutex_cleanup(void)
+static void accept_mutex_cleanup(void *foo)
{
unlink(lock_fname);
}
@@ -637,13 +622,13 @@
exit(1);
}
}
+
/*
* Initialize mutex lock.
* Must be safe to call this on a restart.
*/
static void accept_mutex_init(pool *p)
{
-
expand_lock_fname(p);
unlink(lock_fname);
lock_fd = popenf(p, lock_fname, O_CREAT | O_WRONLY | O_EXCL, 0600);
@@ -652,6 +637,7 @@
"Parent cannot open lock file: %s\n", lock_fname);
exit(1);
}
+ register_cleanup(p, NULL, accept_mutex_cleanup, null_cleanup);
}
static void accept_mutex_on(void)
@@ -685,7 +671,6 @@
/* Multithreaded systems don't complete between processes for
* the sockets. */
#define NO_SERIALIZED_ACCEPT
-#define accept_mutex_cleanup()
#define accept_mutex_child_init(x)
#define accept_mutex_init(x)
#define accept_mutex_on()
@@ -754,8 +739,8 @@
/* a clean exit from a child with proper cleanup */
static void __attribute__((noreturn)) clean_child_exit(int code)
{
- child_exit_modules(pconf, server_conf);
- destroy_pool(pconf);
+ child_exit_modules(pchild, server_conf);
+ destroy_pool(pchild);
exit(code);
}
@@ -1293,7 +1278,14 @@
#else /* MULTITHREAD */
#if defined(HAVE_MMAP)
-static void setup_shared_mem(void)
+#ifdef QNX
+static void cleanup_shared_mem(void *d)
+{
+ shm_unlink(scoreboard_fname);
+}
+#endif
+
+static void setup_shared_mem(pool *p)
{
caddr_t m;
@@ -1360,6 +1352,7 @@
exit(1);
}
close(fd);
+ register_cleanup(p, NULL, cleanup_shared_mem, null_cleanup);
#elif defined(MAP_ANON) || defined(MAP_FILE)
/* BSD style */
@@ -1416,7 +1409,7 @@
static key_t shmkey = IPC_PRIVATE;
static int shmid = -1;
-static void setup_shared_mem(void)
+static void setup_shared_mem(pool *p)
{
struct shmid_ds shmbuf;
#ifdef MOVEBREAK
@@ -1536,6 +1529,11 @@
return rv < 0 ? rv : orig_sz - bufsz;
}
+
+static void cleanup_scoreboard_file(void *foo)
+{
+ unlink(scoreboard_fname);
+}
#endif
/* Called by parent process */
@@ -1547,7 +1545,7 @@
#ifndef SCOREBOARD_FILE
if (scoreboard_image == NULL) {
- setup_shared_mem();
+ setup_shared_mem(p);
}
memset(scoreboard_image, 0, SCOREBOARD_SIZE);
scoreboard_image->global.exit_generation = exit_gen;
@@ -1561,6 +1559,7 @@
fprintf(stderr, "Cannot open scoreboard file:\n");
exit(1);
}
+ register_cleanup(p, NULL, cleanup_scoreboard_file, null_cleanup);
memset((char *) scoreboard_image, 0, sizeof(*scoreboard_image));
scoreboard_image->global.exit_generation = exit_gen;
@@ -1600,15 +1599,6 @@
#endif
}
-void cleanup_scoreboard(void)
-{
-#ifdef SCOREBOARD_FILE
- unlink(scoreboard_fname);
-#elif defined(QNX) && defined(HAVE_MMAP)
- shm_unlink(scoreboard_fname);
-#endif
-}
-
/* Routines called to deal with the scoreboard image
* --- note that we do *not* need write locks, since update_child_status
* only updates a *single* record in place, and only one process writes to
@@ -1644,6 +1634,14 @@
#endif
}
+/* a clean exit from the parent with proper cleanup */
+static void __attribute__((noreturn)) clean_parent_exit(int code)
+{
+ /* Clear the pool - including any registered cleanups */
+ destroy_pool(pconf);
+ exit(code);
+}
+
int update_child_status(int child_num, int status, request_rec *r)
{
int old_status;
@@ -2695,9 +2693,14 @@
my_child_num = child_num_arg;
requests_this_child = 0;
+ /* Get a sub pool for global allocations in this child, so that
+ * we can have cleanups occur when the child exits.
+ */
+ pchild = make_sub_pool(pconf);
+
/* needs to be done before we switch UIDs so we have permissions */
- reopen_scoreboard(pconf);
- SAFE_ACCEPT(accept_mutex_child_init(pconf));
+ reopen_scoreboard(pchild);
+ SAFE_ACCEPT(accept_mutex_child_init(pchild));
#ifdef MPE
/* Only try to switch if we're running as MANAGER.SYS */
@@ -2720,7 +2723,7 @@
}
#endif
- child_init_modules(pconf, server_conf);
+ child_init_modules(pchild, server_conf);
(void) update_child_status(my_child_num, SERVER_READY, (request_rec *) NULL);
@@ -3223,6 +3226,7 @@
}
#ifdef SCOREBOARD_FILE
else if (scoreboard_fd != -1) {
+ kill_cleanup(pconf, NULL, cleanup_scoreboard_file);
kill_cleanups_for_fd(pconf, scoreboard_fd);
}
#endif
@@ -3354,12 +3358,7 @@
aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE, server_conf,
"httpd: caught SIGTERM, shutting down");
- /* Clear the pool - including any registered cleanups */
- destroy_pool(pconf);
- cleanup_scoreboard();
- SAFE_ACCEPT(accept_mutex_cleanup());
-
- exit(0);
+ clean_parent_exit(0);
}
/* we've been told to restart */
@@ -3368,7 +3367,7 @@
if (one_process) {
/* not worth thinking about */
- exit(0);
+ clean_parent_exit(0);
}
if (is_graceful) {
@@ -3412,9 +3411,6 @@
"SIGHUP received. Attempting to restart");
}
++generation;
-
- SAFE_ACCEPT(accept_mutex_cleanup());
-
} while (restart_pending);
} /* standalone_main */
@@ -4097,8 +4093,6 @@
cleanup_scoreboard();
exit(0);
-
-
} /* standalone_main */
Re: [PATCH] pool confusion (take 1)
Posted by Dean Gaudet <dg...@arctic.org>.
On Mon, 10 Nov 1997, Alexei Kosut wrote:
> It has to be performed in *both* the child and the parent, because
> they *both* have a copy of pconf. Seperate copies. Calling a cleanup
> on one doesn't clean up the other (unless you're suggesting that pconf
> should be made shared memory, which I highly doubt you are).
No, consider fopen(). Notice how when you clean that up it's cleaned up
differently due to a fork() (which is about to exec()) than it is due to
the pool being cleaned. I'm saying the same thing exists between the
parent and child. There are cleanups that need to be performed exactly
once, and that exactly once has to be when the parent restarts.
For example, piped logs. They're registered in pconf, and MUST NOT be
cleaned by the child because the resource they register is another pid;
and the cleanup they do is to kill off that pid. Note that the children
don't get a copy of this other task, they share the same task with the
parent.
Another example is flock(). Prior to this patch it couldn't use the
register_cleanup mechanism to clean up its resource because the cleanup
must be run exactly once; by the parent.
> > > Admittedly, the subsequent exit() does get rid of the said memory, but
> > > this is the only example I can think of off the top of my head.
> > >
> > > I am convinced, however, that it is neccessary in some cases.
> >
> > This is what pchild is for. If something needs to be cleaned up when the
> > child exits, then it's probably only created when the child starts. One
>
> "probably." I'm saying that this isn't always true. I buy your
> rationale that many things only should be cleaned up by the parent,
> but not everything. If you're going to do this, then I suggest
> creating pchild at the same time as pconf. That way, you always have
> an appropriate pool to use.
There is nothing stopping a module from registering something in pconf
during init(), and re-registering the same/similar cleanup in pchild
during child_init(). So the functionality is already there. I don't want
pconf and pchild to be available outside http_main; they're passed to
init_modules() and child_init_modules(). That's the thread-safe way to do
things... globals suck. I'm not sure why pconf is global currently...
Dean
Re: [PATCH] pool confusion (take 1)
Posted by Alexei Kosut <ak...@nueva.pvt.k12.ca.us>.
On Mon, 10 Nov 1997, Dean Gaudet wrote:
>
>
> On Sun, 9 Nov 1997, Alexei Kosut wrote:
>
> > What I'm trying to point out is this: Let's say the main server does a
> > pregcomp() with pconf as the pool. This sets up a cleanup in pconf to
> > get rid of the regex once it's done with (it calls free()). Now, if
> > the main server now spawns a child, that child should certainly want
> > to call that cleanup, since it now has its own copy of the memory
> > that's being freed.
>
> This is an example of a cleanup that can be performed in the parent, i.e.
> pconf. It has to be performed in the parent because of restarts.
It has to be performed in *both* the child and the parent, because
they *both* have a copy of pconf. Seperate copies. Calling a cleanup
on one doesn't clean up the other (unless you're suggesting that pconf
should be made shared memory, which I highly doubt you are).
> > Admittedly, the subsequent exit() does get rid of the said memory, but
> > this is the only example I can think of off the top of my head.
> >
> > I am convinced, however, that it is neccessary in some cases.
>
> This is what pchild is for. If something needs to be cleaned up when the
> child exits, then it's probably only created when the child starts. One
"probably." I'm saying that this isn't always true. I buy your
rationale that many things only should be cleaned up by the parent,
but not everything. If you're going to do this, then I suggest
creating pchild at the same time as pconf. That way, you always have
an appropriate pool to use.
> thing you missed in your explanation of fork() was how the non-memory
> resources are handled. flock()'s stuff is an example of something that
> has to be created in the child, and cleaned in the child. In addition to
> having a different creation/cleanup in the parent.
-- Alexei Kosut <ak...@nueva.pvt.k12.ca.us>
Re: [PATCH] pool confusion (take 1)
Posted by Dean Gaudet <dg...@arctic.org>.
On Sun, 9 Nov 1997, Alexei Kosut wrote:
> What I'm trying to point out is this: Let's say the main server does a
> pregcomp() with pconf as the pool. This sets up a cleanup in pconf to
> get rid of the regex once it's done with (it calls free()). Now, if
> the main server now spawns a child, that child should certainly want
> to call that cleanup, since it now has its own copy of the memory
> that's being freed.
This is an example of a cleanup that can be performed in the parent, i.e.
pconf. It has to be performed in the parent because of restarts.
> Admittedly, the subsequent exit() does get rid of the said memory, but
> this is the only example I can think of off the top of my head.
>
> I am convinced, however, that it is neccessary in some cases.
This is what pchild is for. If something needs to be cleaned up when the
child exits, then it's probably only created when the child starts. One
thing you missed in your explanation of fork() was how the non-memory
resources are handled. flock()'s stuff is an example of something that
has to be created in the child, and cleaned in the child. In addition to
having a different creation/cleanup in the parent.
Dean
Re: [PATCH] pool confusion (take 1)
Posted by Alexei Kosut <ak...@nueva.pvt.k12.ca.us>.
On Sun, 9 Nov 1997, Dean Gaudet wrote:
> The current code has two related bugs:
>
> - flock doesn't seem to be cleaning up its file on
> SIGTERM under Linux
> - the register_other_child API is getting "unregister"
> events when it shouldn't (this one is on the STATUS already)
>
> They're related because what's happened is that all httpd children are
> calling destroy_pool(pconf). This is wrong.
>
> Alexei added this when he was fixing up the child_exit API in August, you
> can read that thread if you want to know what he did... but he made the
> children start calling destroy_pool(pconf) claiming it was the right thing
> to do. I'm now claiming it's not the right thing to do.
Not in the instance you're reffering to, no. But it is sometimes a
good idea. To see why, recall what happens when a process forks in
Unix (and this isn't really my area of expertise, so if I make a
mistake, please let me know). Basically, the memory segment occupied
by the process is copied, byte for byte, into a new process, and then
control is returned to each process, with some indication of which
process they are. Things that are internal to the process, like
memory, there are now two copies of. Things that are external, like
file handles (which are really just numbers that mean something to the
system) are not.
In other words, it's sort of like what would happen if I cloned you
(and I mean the "stick you in a Xerox machine" type of clone that one
finds in movies, not the biological type of clone). Each of you has
your own liver, your own legs, your own brain, which are identical but
completely independent. However, you both have the same house, the
same car, the same driver's license number.
What I'm trying to point out is this: Let's say the main server does a
pregcomp() with pconf as the pool. This sets up a cleanup in pconf to
get rid of the regex once it's done with (it calls free()). Now, if
the main server now spawns a child, that child should certainly want
to call that cleanup, since it now has its own copy of the memory
that's being freed.
Admittedly, the subsequent exit() does get rid of the said memory, but
this is the only example I can think of off the top of my head.
I am convinced, however, that it is neccessary in some cases.
-- Alexei Kosut <ak...@nueva.pvt.k12.ca.us>
Re: [PATCH] pool confusion (take 2)
Posted by Dean Gaudet <dg...@arctic.org>.
On Sun, 9 Nov 1997, Dean Gaudet wrote:
> Remaining to do:
> - test Solaris with pthread, mmap
Roy tested and found one syntax mistake I made, I've included the fix
here. In fact that's the only change in this take.
> - test IRIX with uslock if I get motivated enough
Still untested; leave it for beta.
> - figure out what other signals we can trap and rationally clean up on;
> this is important -- we can clean up on a lot of other signals,
> like SIGXCPU, and SIGFPE. We should do that cleanup because we
> have a few configurations in which cleanups are important.
> (i.e. pthreads, flock, sysvsem, qnx mmap, file-based scoreboard)
Would be nice, but can wait until 1.3b4 I hope. Because I'm short on time
this week, and I'd like to see 1.3b3 out friday.
> - win32 -- I figure someone else can do this after commit, it already has
> a pchild that I think has the same meaning
I don't think I broke win32 ... I just think the semantics might be
slightly wrong; not a showstopper.
Dean
Index: alloc.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/alloc.c,v
retrieving revision 1.57
diff -u -r1.57 alloc.c
--- alloc.c 1997/11/06 21:54:07 1.57
+++ alloc.c 1997/11/11 19:59:31
@@ -936,6 +936,11 @@
#endif /* ndef WIN32 */
}
+API_EXPORT(void) null_cleanup(void *data)
+{
+ /* do nothing cleanup routine */
+}
+
/*****************************************************************
*
* Files and file descriptors; these are just an application of the
Index: alloc.h
===================================================================
RCS file: /export/home/cvs/apachen/src/main/alloc.h,v
retrieving revision 1.36
diff -u -r1.36 alloc.h
--- alloc.h 1997/10/26 21:14:16 1.36
+++ alloc.h 1997/11/11 19:59:31
@@ -194,6 +194,10 @@
API_EXPORT(void) kill_cleanup(pool *p, void *data, void (*plain_cleanup) (void *));
API_EXPORT(void) run_cleanup(pool *p, void *data, void (*cleanup) (void *));
+/* A "do-nothing" cleanup, for register_cleanup; it's faster to do
+ * things this way than to test for NULL. */
+API_EXPORT(void) null_cleanup(void *data);
+
/* The time between when a resource is actually allocated, and when it
* its cleanup is registered is a critical section, during which the
* resource could leak if we got interrupted or timed out. So, anything
Index: http_main.c
===================================================================
RCS file: /export/home/cvs/apachen/src/main/http_main.c,v
retrieving revision 1.247
diff -u -r1.247 http_main.c
--- http_main.c 1997/11/08 19:19:15 1.247
+++ http_main.c 1997/11/11 19:59:33
@@ -242,6 +242,7 @@
pool *pconf; /* Pool for config stuff */
pool *ptrans; /* Pool for per-transaction stuff */
+pool *pchild; /* Pool for httpd child stuff */
int APACHE_TLS my_pid; /* it seems silly to call getpid all the time */
#ifndef MULTITHREAD
@@ -278,8 +279,6 @@
static ulock_t uslock = NULL;
-#define accept_mutex_cleanup()
-
#define accept_mutex_child_init(x)
static void accept_mutex_init(pool *p)
@@ -351,7 +350,7 @@
static sigset_t accept_block_mask;
static sigset_t accept_previous_mask;
-static void accept_mutex_child_cleanup(void *data)
+static void accept_mutex_child_cleanup(void *foo)
{
if (accept_mutex != (void *)(caddr_t)-1
&& have_accept_mutex) {
@@ -359,7 +358,12 @@
}
}
-static void accept_mutex_cleanup(void)
+static void accept_mutex_child_init(pool *p)
+{
+ register_cleanup(p, NULL, accept_mutex_child_cleanup, null_cleanup);
+}
+
+static void accept_mutex_cleanup(void *foo)
{
if (accept_mutex != (void *)(caddr_t)-1
&& munmap((caddr_t) accept_mutex, sizeof(*accept_mutex))) {
@@ -368,8 +372,6 @@
accept_mutex = (void *)(caddr_t)-1;
}
-#define accept_mutex_child_init(x)
-
static void accept_mutex_init(pool *p)
{
pthread_mutexattr_t mattr;
@@ -403,8 +405,7 @@
sigdelset(&accept_block_mask, SIGHUP);
sigdelset(&accept_block_mask, SIGTERM);
sigdelset(&accept_block_mask, SIGUSR1);
- register_cleanup(pconf, NULL, accept_mutex_child_cleanup,
- accept_mutex_child_cleanup);
+ register_cleanup(p, NULL, accept_mutex_cleanup, null_cleanup);
}
static void accept_mutex_on()
@@ -462,26 +463,20 @@
#endif
-static int sem_cleanup_registered;
-static pid_t sem_cleanup_pid;
static int sem_id = -1;
static struct sembuf op_on;
static struct sembuf op_off;
/* We get a random semaphore ... the lame sysv semaphore interface
* means we have to be sure to clean this up or else we'll leak
- * semaphores. Note that this is registered via atexit, so we can't
- * do many things in here... especially nothing that would allocate
- * memory or use a FILE *.
+ * semaphores.
*/
-static void accept_mutex_cleanup(void)
+static void accept_mutex_cleanup(void *foo)
{
union semun ick;
if (sem_id < 0)
return;
- if (getpid() != sem_cleanup_pid)
- return;
/* this is ignored anyhow */
ick.val = 0;
semctl(sem_id, 0, IPC_RMID, ick);
@@ -494,15 +489,6 @@
union semun ick;
struct semid_ds buf;
- if (!sem_cleanup_registered) {
- /* only the parent will try to do cleanup */
- sem_cleanup_pid = getpid();
- if (atexit(accept_mutex_cleanup)) {
- perror("atexit");
- exit(1);
- }
- sem_cleanup_registered = 1;
- }
/* acquire the semaphore */
sem_id = semget(IPC_PRIVATE, 1, IPC_CREAT | 0600);
if (sem_id < 0) {
@@ -527,6 +513,7 @@
exit(1);
}
}
+ register_cleanup(p, NULL, accept_mutex_cleanup, null_cleanup);
/* pre-initialize these */
op_on.sem_num = 0;
@@ -559,8 +546,6 @@
static int lock_fd = -1;
-#define accept_mutex_cleanup()
-
#define accept_mutex_child_init(x)
/*
@@ -618,7 +603,7 @@
static int lock_fd = -1;
-static void accept_mutex_cleanup(void)
+static void accept_mutex_cleanup(void *foo)
{
unlink(lock_fname);
}
@@ -637,13 +622,13 @@
exit(1);
}
}
+
/*
* Initialize mutex lock.
* Must be safe to call this on a restart.
*/
static void accept_mutex_init(pool *p)
{
-
expand_lock_fname(p);
unlink(lock_fname);
lock_fd = popenf(p, lock_fname, O_CREAT | O_WRONLY | O_EXCL, 0600);
@@ -652,6 +637,7 @@
"Parent cannot open lock file: %s\n", lock_fname);
exit(1);
}
+ register_cleanup(p, NULL, accept_mutex_cleanup, null_cleanup);
}
static void accept_mutex_on(void)
@@ -685,7 +671,6 @@
/* Multithreaded systems don't complete between processes for
* the sockets. */
#define NO_SERIALIZED_ACCEPT
-#define accept_mutex_cleanup()
#define accept_mutex_child_init(x)
#define accept_mutex_init(x)
#define accept_mutex_on()
@@ -754,8 +739,8 @@
/* a clean exit from a child with proper cleanup */
static void __attribute__((noreturn)) clean_child_exit(int code)
{
- child_exit_modules(pconf, server_conf);
- destroy_pool(pconf);
+ child_exit_modules(pchild, server_conf);
+ destroy_pool(pchild);
exit(code);
}
@@ -1293,7 +1278,14 @@
#else /* MULTITHREAD */
#if defined(HAVE_MMAP)
-static void setup_shared_mem(void)
+#ifdef QNX
+static void cleanup_shared_mem(void *d)
+{
+ shm_unlink(scoreboard_fname);
+}
+#endif
+
+static void setup_shared_mem(pool *p)
{
caddr_t m;
@@ -1360,6 +1352,7 @@
exit(1);
}
close(fd);
+ register_cleanup(p, NULL, cleanup_shared_mem, null_cleanup);
#elif defined(MAP_ANON) || defined(MAP_FILE)
/* BSD style */
@@ -1416,7 +1409,7 @@
static key_t shmkey = IPC_PRIVATE;
static int shmid = -1;
-static void setup_shared_mem(void)
+static void setup_shared_mem(pool *p)
{
struct shmid_ds shmbuf;
#ifdef MOVEBREAK
@@ -1536,6 +1529,11 @@
return rv < 0 ? rv : orig_sz - bufsz;
}
+
+static void cleanup_scoreboard_file(void *foo)
+{
+ unlink(scoreboard_fname);
+}
#endif
/* Called by parent process */
@@ -1547,7 +1545,7 @@
#ifndef SCOREBOARD_FILE
if (scoreboard_image == NULL) {
- setup_shared_mem();
+ setup_shared_mem(p);
}
memset(scoreboard_image, 0, SCOREBOARD_SIZE);
scoreboard_image->global.exit_generation = exit_gen;
@@ -1561,6 +1559,7 @@
fprintf(stderr, "Cannot open scoreboard file:\n");
exit(1);
}
+ register_cleanup(p, NULL, cleanup_scoreboard_file, null_cleanup);
memset((char *) scoreboard_image, 0, sizeof(*scoreboard_image));
scoreboard_image->global.exit_generation = exit_gen;
@@ -1600,15 +1599,6 @@
#endif
}
-void cleanup_scoreboard(void)
-{
-#ifdef SCOREBOARD_FILE
- unlink(scoreboard_fname);
-#elif defined(QNX) && defined(HAVE_MMAP)
- shm_unlink(scoreboard_fname);
-#endif
-}
-
/* Routines called to deal with the scoreboard image
* --- note that we do *not* need write locks, since update_child_status
* only updates a *single* record in place, and only one process writes to
@@ -1644,6 +1634,14 @@
#endif
}
+/* a clean exit from the parent with proper cleanup */
+static void __attribute__((noreturn)) clean_parent_exit(int code)
+{
+ /* Clear the pool - including any registered cleanups */
+ destroy_pool(pconf);
+ exit(code);
+}
+
int update_child_status(int child_num, int status, request_rec *r)
{
int old_status;
@@ -2695,9 +2693,14 @@
my_child_num = child_num_arg;
requests_this_child = 0;
+ /* Get a sub pool for global allocations in this child, so that
+ * we can have cleanups occur when the child exits.
+ */
+ pchild = make_sub_pool(pconf);
+
/* needs to be done before we switch UIDs so we have permissions */
- reopen_scoreboard(pconf);
- SAFE_ACCEPT(accept_mutex_child_init(pconf));
+ reopen_scoreboard(pchild);
+ SAFE_ACCEPT(accept_mutex_child_init(pchild));
#ifdef MPE
/* Only try to switch if we're running as MANAGER.SYS */
@@ -2720,7 +2723,7 @@
}
#endif
- child_init_modules(pconf, server_conf);
+ child_init_modules(pchild, server_conf);
(void) update_child_status(my_child_num, SERVER_READY, (request_rec *) NULL);
@@ -3223,6 +3226,7 @@
}
#ifdef SCOREBOARD_FILE
else if (scoreboard_fd != -1) {
+ kill_cleanup(pconf, NULL, cleanup_scoreboard_file);
kill_cleanups_for_fd(pconf, scoreboard_fd);
}
#endif
@@ -3354,12 +3358,7 @@
aplog_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE, server_conf,
"httpd: caught SIGTERM, shutting down");
- /* Clear the pool - including any registered cleanups */
- destroy_pool(pconf);
- cleanup_scoreboard();
- SAFE_ACCEPT(accept_mutex_cleanup());
-
- exit(0);
+ clean_parent_exit(0);
}
/* we've been told to restart */
@@ -3368,7 +3367,7 @@
if (one_process) {
/* not worth thinking about */
- exit(0);
+ clean_parent_exit(0);
}
if (is_graceful) {
@@ -3412,9 +3411,6 @@
"SIGHUP received. Attempting to restart");
}
++generation;
-
- SAFE_ACCEPT(accept_mutex_cleanup());
-
} while (restart_pending);
} /* standalone_main */
@@ -4097,8 +4093,6 @@
cleanup_scoreboard();
exit(0);
-
-
} /* standalone_main */