You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Bill Stoddard <st...@raleigh.ibm.com> on 2000/02/03 16:41:36 UTC
[PATCH] Associating unload_module cleanup with the global pool rather than pconf
If I hear no objections, I will commit this patch later today.
Bill
cvs diff -u mod_so.c
Index: mod_so.c
===================================================================
RCS file: /home/cvs/apache-2.0/src/modules/standard/mod_so.c,v
retrieving revision 1.7
diff -u -r1.7 mod_so.c
--- mod_so.c 2000/01/21 19:24:46 1.7
+++ mod_so.c 2000/02/03 15:38:50
@@ -276,7 +276,7 @@
* we do a restart (or shutdown) this cleanup will cause the
* shared object to be unloaded.
*/
- ap_register_cleanup(cmd->pool, modi,
+ ap_register_cleanup(g_pHookPool, modi,
(ap_status_t (*)(void*))unload_module, ap_null_cleanup);
/*
________________________________________________
Bill Stoddard stoddard@raleigh.ibm.com
Come to the first official Apache Software Foundation
Conference! <http://ApacheCon.Com/>
Re: [PATCH] Associating unload_module cleanup with the global poolrather
than pconf
Posted by "Paul J. Reder" <re...@raleigh.ibm.com>.
Bill,
Some of the modules had to check for which pass of init they were
in. I know its ugly, but some of the modules (like mod_rewrite)
start external programs or allocate structures and fill in
pointers. At the very least these modules must kill these external
programs and/or clean up the pointers between init phases. The
cleanest most efficient way seems to be to only start the process/
allocate the memory on the second pass. The phase check was added
recently to mod_rewrite to fix some initialization errors.
--
Paul J. Reder
---------------------------------------------------------------
No one can make you feel inferior without your consent.
-- Eleanor Roosevelt
Re: [PATCH] Associating unload_module cleanup with the global poolrather than pconf
Posted by Bill Stoddard <st...@raleigh.ibm.com>.
Thanks for the reply. Yea, I had convinced myself this wasn't a good idea
but not for the reasons you bring up. All this patch does is cause the
module unload to occur when the global pool is destroyed (and not when the
pconf pool is destroyed). This patch, however, does not really fix the
problems I am seeing with hooking dynamically loaded modules. Here is the
problem:
ap_run_pre_config() - 1st call, does not pick-up DSOs because they have not
been loaded yet
ap_read_config() - 1st call
1. reads the config and loads DSOs specified in the config file via
LoadModule directives
2. registers the DSO hooks (pre_config, et. al) and stores info
about entry points, etc in a table allocated out of the global process pool
ap_clear_pool(pconf) - this unloads the DSOs because of the module_cleanup
registered against the pconf pool
ap_run_pre_config() - 2nd call. ap_run_pre_config determines there are hooks
to run and tries to call them. The problem is the DSO's are now unloaded,
resulting in a seg fault
ap_read_config() - 2nd call
1. reads the config and loads the DSOs
2. registers the DSO hooks.
ap_run_mpm()
If I register the module clean-up with the global process pool, we avoid the
seg fault, but we still loose the knowledge that the modules have already
been loaded once (because that info is saved in pconf, not the global pool).
So, we either need to save all the loaded module info in the global pool or
save the module hooks info in pconf. Some of the modules have a static
once_through variable defined to prevent them from allocating resources on
the first call to the hooks. This stinks to high heaven. On a closely
related topic, it would be nice to be able to not call the module hooks (or
at least some of them) in the parent process on platforms that do not
support fork.
Bill
----- Original Message -----
From: <rb...@apache.org>
To: <ne...@apache.org>
Sent: Thursday, February 03, 2000 11:46 AM
Subject: Re: [PATCH] Associating unload_module cleanup with the global
poolrather than pconf
>
> I think this is bad, but I'm not sure. We read the config twice, and in
> between, we are clearing the pconf pool. If we register the module
> cleanup in the global pool, then when we clear the pconf pool, we'll have
> a memory leak, right? I may be missing something, but I don't think I am.
> I think we are better off allocating the pre_config stuff from pconf. The
> pre_config step just means we are doing it before we read the config, to
> my way of thinking, the pre-config step is still part of the configuration
> and should be done in pconf.
>
> Again, this is all without readng through the code, but I am -1 for this
> patch until the memory leak is cleared up in my mind. :-)
>
> Ryan
>
> On Thu, 3 Feb 2000, Bill Stoddard wrote:
>
> > If I hear no objections, I will commit this patch later today.
> >
> > Bill
> > cvs diff -u mod_so.c
> > Index: mod_so.c
> > ===================================================================
> > RCS file: /home/cvs/apache-2.0/src/modules/standard/mod_so.c,v
> > retrieving revision 1.7
> > diff -u -r1.7 mod_so.c
> > --- mod_so.c 2000/01/21 19:24:46 1.7
> > +++ mod_so.c 2000/02/03 15:38:50
> > @@ -276,7 +276,7 @@
> > * we do a restart (or shutdown) this cleanup will cause the
> > * shared object to be unloaded.
> > */
> > - ap_register_cleanup(cmd->pool, modi,
> > + ap_register_cleanup(g_pHookPool, modi,
> > (ap_status_t (*)(void*))unload_module, ap_null_cleanup);
> >
> > /*
> >
> >
> > ________________________________________________
> > Bill Stoddard stoddard@raleigh.ibm.com
> >
> > Come to the first official Apache Software Foundation
> > Conference! <http://ApacheCon.Com/>
> >
> >
> >
> >
>
>
> Come to the first official Apache Software Foundation
> Conference!!! <http://ApacheCon.Com/>
>
>
____________________________________________________________________________
___
> Ryan Bloom rbb@ntrnet.net
> 2121 Stonehenge Dr. Apt #3
> Raleigh, NC 27615 Ryan Bloom -- thinker, adventurer, artist,
> writer, but mostly, friend.
> --------------------------------------------------------------------------
-----
>
>
Re: [PATCH] Associating unload_module cleanup with the global pool
rather than pconf
Posted by rb...@apache.org.
I think this is bad, but I'm not sure. We read the config twice, and in
between, we are clearing the pconf pool. If we register the module
cleanup in the global pool, then when we clear the pconf pool, we'll have
a memory leak, right? I may be missing something, but I don't think I am.
I think we are better off allocating the pre_config stuff from pconf. The
pre_config step just means we are doing it before we read the config, to
my way of thinking, the pre-config step is still part of the configuration
and should be done in pconf.
Again, this is all without readng through the code, but I am -1 for this
patch until the memory leak is cleared up in my mind. :-)
Ryan
On Thu, 3 Feb 2000, Bill Stoddard wrote:
> If I hear no objections, I will commit this patch later today.
>
> Bill
> cvs diff -u mod_so.c
> Index: mod_so.c
> ===================================================================
> RCS file: /home/cvs/apache-2.0/src/modules/standard/mod_so.c,v
> retrieving revision 1.7
> diff -u -r1.7 mod_so.c
> --- mod_so.c 2000/01/21 19:24:46 1.7
> +++ mod_so.c 2000/02/03 15:38:50
> @@ -276,7 +276,7 @@
> * we do a restart (or shutdown) this cleanup will cause the
> * shared object to be unloaded.
> */
> - ap_register_cleanup(cmd->pool, modi,
> + ap_register_cleanup(g_pHookPool, modi,
> (ap_status_t (*)(void*))unload_module, ap_null_cleanup);
>
> /*
>
>
> ________________________________________________
> Bill Stoddard stoddard@raleigh.ibm.com
>
> Come to the first official Apache Software Foundation
> Conference! <http://ApacheCon.Com/>
>
>
>
>
Come to the first official Apache Software Foundation
Conference!!! <http://ApacheCon.Com/>
_______________________________________________________________________________
Ryan Bloom rbb@ntrnet.net
2121 Stonehenge Dr. Apt #3
Raleigh, NC 27615 Ryan Bloom -- thinker, adventurer, artist,
writer, but mostly, friend.
-------------------------------------------------------------------------------