This is mostly an APR patch but it fixes a symptom (segfault)
associated with mod_cgid after doing "apachectl graceful" followed by
"apachectl restart".
I turned on Greg Stein's mprotect-based memory debugging to help test
it out.
Before, we were leaving registrations in the other_children list after
the pool from which the list element was allocated was destroyed.
With this patch, we use the normal APR auto-cleanup design ith the
other-child registration so that we don't leave the registration
dangling.
The cleanup is a bit hokey because of the API; because
apr_proc_other_child_unregister() is given a user data ptr instead of
an apr_foo_t, we have to search the private list of registrations to
find the pool with which the cleanup is associated.
With this patch it takes a couple of secs for mod_cgid to go away, so
maybe we should be doing an explicit unregister somewhere, but at
least the auto unregister seems to work.
I have pretty low confidence in my understanding of anything to do
with APR other-child code, so I look forward to being pointed in the
right direction.
Index: modules/generators/mod_cgid.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/generators/mod_cgid.c,v
retrieving revision 1.83
diff -u -r1.83 mod_cgid.c
--- modules/generators/mod_cgid.c 2001/04/03 19:12:14 1.83
+++ modules/generators/mod_cgid.c 2001/04/26 22:34:49
@@ -242,7 +242,7 @@
apr_proc_other_child_unregister(data);
break;
case APR_OC_REASON_UNREGISTER:
- apr_pool_destroy(pcgi);
+ /* don't apr_pool_destroy(pcgi); its cleanup drives this path */
kill(*sd, SIGHUP);
break;
}
Index: srclib/apr/include/arch/unix/misc.h
===================================================================
RCS file: /home/cvspublic/apr/include/arch/unix/misc.h,v
retrieving revision 1.25
diff -u -r1.25 misc.h
--- srclib/apr/include/arch/unix/misc.h 2001/02/25 20:39:32 1.25
+++ srclib/apr/include/arch/unix/misc.h 2001/04/26 22:35:13
@@ -88,6 +88,7 @@
#endif
struct apr_other_child_rec_t {
+ apr_pool_t *p;
struct apr_other_child_rec_t *next;
int id; /* This is either a pid or tid depending on the platform */
void (*maintenance) (int, void *, int);
Index: srclib/apr/misc/unix/otherchild.c
===================================================================
RCS file: /home/cvspublic/apr/misc/unix/otherchild.c,v
retrieving revision 1.20
diff -u -r1.20 otherchild.c
--- srclib/apr/misc/unix/otherchild.c 2001/02/16 04:15:56 1.20
+++ srclib/apr/misc/unix/otherchild.c 2001/04/26 22:35:17
@@ -71,6 +71,22 @@
static apr_other_child_rec_t *other_children = NULL;
+static apr_status_t other_child_cleanup(void *data)
+{
+ apr_other_child_rec_t **pocr, *nocr;
+
+ for (pocr = &other_children; *pocr; pocr = &(*pocr)->next) {
+ if ((*pocr)->data == data) {
+ nocr = (*pocr)->next;
+ (*(*pocr)->maintenance) (APR_OC_REASON_UNREGISTER, (*pocr)->data,
-1);
+ *pocr = nocr;
+ /* XXX: um, well we've just wasted some space in pconf ? */
+ return APR_SUCCESS;
+ }
+ }
+ return APR_SUCCESS;
+}
+
APR_DECLARE(void) apr_proc_other_child_register(apr_proc_t *pid,
void (*maintenance) (int reason, void *, int status),
void *data, apr_file_t *write_fd, apr_pool_t *p)
@@ -78,6 +94,7 @@
apr_other_child_rec_t *ocr;
ocr = apr_palloc(p, sizeof(*ocr));
+ ocr->p = p;
ocr->id = pid->pid;
ocr->maintenance = maintenance;
ocr->data = data;
@@ -89,21 +106,25 @@
}
ocr->next = other_children;
other_children = ocr;
+ apr_pool_cleanup_register(p, ocr->data, other_child_cleanup,
+ apr_pool_cleanup_null);
}
APR_DECLARE(void) apr_proc_other_child_unregister(void *data)
{
- apr_other_child_rec_t **pocr, *nocr;
+ apr_other_child_rec_t *cur;
- for (pocr = &other_children; *pocr; pocr = &(*pocr)->next) {
- if ((*pocr)->data == data) {
- nocr = (*pocr)->next;
- (*(*pocr)->maintenance) (APR_OC_REASON_UNREGISTER, (*pocr)->data,
-1);
- *pocr = nocr;
- /* XXX: um, well we've just wasted some space in pconf ? */
- return;
+ cur = other_children;
+ while (cur) {
+ if (cur->data == data) {
+ break;
}
+ cur = cur->next;
}
+
+ /* segfault if this function called with invalid parm */
+ apr_pool_cleanup_kill(cur->p, cur->data, other_child_cleanup);
+ other_child_cleanup(data);
}
/* test to ensure that the write_fds are all still writable, otherwise
--
Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
