You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by rb...@apache.org on 2002/03/20 06:31:54 UTC

Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c

On 20 Mar 2002 wrowe@apache.org wrote:

> wrowe       02/03/19 20:29:55
> 
>   Modified:    server/mpm/winnt mpm_winnt.c
>   Log:
>     When restarting [always graceful on Win32], we don't repeat pre_mpm
>     (Unix doesn't, we shouldn't either.)  [Ryan Bloom]
>    
>   -    if ((parent_pid == my_pid) || one_process) {
>   +    /* ### If non-graceful restarts are ever introduced - we need to rerun 
>   +     * the pre_mpm hook on subsequent non-graceful restarts.  But Win32 
>   +     * has only graceful style restarts - and we need this hook to act 
>   +     * the same on Win32 as on Unix.
>   +     */
>   +    if (!restart && ((parent_pid == my_pid) || one_process)) {
>            /* Set up the scoreboard. */
>            if (ap_run_pre_mpm(pconf, SB_SHARED) != OK) {
>                return 1;

While I agree with this patch, you also need to kill the cleanup on the
scoreboard, so that it isn't set to NULL when pconf is cleared.  And, you
need to find some way to pass the scoreboard back to the child process
because you are about to start a new one.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
550 Jean St
Oakland CA 94610
-------------------------------------------------------------------------------


Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c

Posted by Jeff Trawick <tr...@attglobal.net>.
Jeff Trawick <tr...@attglobal.net> writes:

> rbb@apache.org writes:
> 
> > On 20 Mar 2002 wrowe@apache.org wrote:
> > 
> > > wrowe       02/03/19 20:29:55
> > > 
> > >   Modified:    server/mpm/winnt mpm_winnt.c
> > >   Log:
> > >     When restarting [always graceful on Win32], we don't repeat pre_mpm
> > >     (Unix doesn't, we shouldn't either.)  [Ryan Bloom]
> > >    
> > >   -    if ((parent_pid == my_pid) || one_process) {
> > >   +    /* ### If non-graceful restarts are ever introduced - we need to rerun 
> > >   +     * the pre_mpm hook on subsequent non-graceful restarts.  But Win32 
> > >   +     * has only graceful style restarts - and we need this hook to act 
> > >   +     * the same on Win32 as on Unix.
> > >   +     */
> > >   +    if (!restart && ((parent_pid == my_pid) || one_process)) {
> > >            /* Set up the scoreboard. */
> > >            if (ap_run_pre_mpm(pconf, SB_SHARED) != OK) {
> > >                return 1;
> > 
> > While I agree with this patch, you also need to kill the cleanup on the
> > scoreboard, so that it isn't set to NULL when pconf is cleared.
> 
> Actually, the cleanup never runs because it never finds the data in
> the pconf pool :)  But yes it needs to be dealt with.

wrong... let me restate...  we currently have logic to kill the
cleanup for graceful restart but that call to cleanup_kill is bogus in
that 1) it searches the wrong pool and 2) if OtherBill's changes to
keep using the same scoreboard forever and ever then we don't need to
kill the cleanup anyway

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c

Posted by Jeff Trawick <tr...@attglobal.net>.
Jeff Trawick <tr...@attglobal.net> writes:

> rbb@apache.org writes:
> 
> > On 20 Mar 2002 wrowe@apache.org wrote:
> > 
> > > wrowe       02/03/19 20:29:55
> > > 
> > >   Modified:    server/mpm/winnt mpm_winnt.c
> > >   Log:
> > >     When restarting [always graceful on Win32], we don't repeat pre_mpm
> > >     (Unix doesn't, we shouldn't either.)  [Ryan Bloom]
> > >    
> > >   -    if ((parent_pid == my_pid) || one_process) {
> > >   +    /* ### If non-graceful restarts are ever introduced - we need to rerun 
> > >   +     * the pre_mpm hook on subsequent non-graceful restarts.  But Win32 
> > >   +     * has only graceful style restarts - and we need this hook to act 
> > >   +     * the same on Win32 as on Unix.
> > >   +     */
> > >   +    if (!restart && ((parent_pid == my_pid) || one_process)) {
> > >            /* Set up the scoreboard. */
> > >            if (ap_run_pre_mpm(pconf, SB_SHARED) != OK) {
> > >                return 1;
> > 
> > While I agree with this patch, you also need to kill the cleanup on the
> > scoreboard, so that it isn't set to NULL when pconf is cleared.
> 
> Actually, the cleanup never runs because it never finds the data in
> the pconf pool :)  But yes it needs to be dealt with.

wrong... let me restate...  we currently have logic to kill the
cleanup for graceful restart but that call to cleanup_kill is bogus in
that 1) it searches the wrong pool and 2) if OtherBill's changes to
keep using the same scoreboard forever and ever then we don't need to
kill the cleanup anyway

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c

Posted by Jeff Trawick <tr...@attglobal.net>.
rbb@apache.org writes:

> On 20 Mar 2002 wrowe@apache.org wrote:
> 
> > wrowe       02/03/19 20:29:55
> > 
> >   Modified:    server/mpm/winnt mpm_winnt.c
> >   Log:
> >     When restarting [always graceful on Win32], we don't repeat pre_mpm
> >     (Unix doesn't, we shouldn't either.)  [Ryan Bloom]
> >    
> >   -    if ((parent_pid == my_pid) || one_process) {
> >   +    /* ### If non-graceful restarts are ever introduced - we need to rerun 
> >   +     * the pre_mpm hook on subsequent non-graceful restarts.  But Win32 
> >   +     * has only graceful style restarts - and we need this hook to act 
> >   +     * the same on Win32 as on Unix.
> >   +     */
> >   +    if (!restart && ((parent_pid == my_pid) || one_process)) {
> >            /* Set up the scoreboard. */
> >            if (ap_run_pre_mpm(pconf, SB_SHARED) != OK) {
> >                return 1;
> 
> While I agree with this patch, you also need to kill the cleanup on the
> scoreboard, so that it isn't set to NULL when pconf is cleared.

Actually, the cleanup never runs because it never finds the data in
the pconf pool :)  But yes it needs to be dealt with.

I was cleaning this issue up this morning (removing cleanups, removing
logic I added recently to restore the proper running_generation field)
and realized (or was confused into thinking) that we aren't handling
non-graceful restart anymore, in that we don't start back with a fresh
scoreboard (fresh except for the running_generation field).  Now,
whatever was in the scoreboard from the previous, whacked generation
of children is still there.

For now I've set aside the cleanup changes until there is some
resolution on how it should be handled (should MPM have to clean up,
should scoreboard-create clean up when it realizes that it doesn't
have to reallocate, etc.).

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c

Posted by Jeff Trawick <tr...@attglobal.net>.
rbb@apache.org writes:

> On 20 Mar 2002 wrowe@apache.org wrote:
> 
> > wrowe       02/03/19 20:29:55
> > 
> >   Modified:    server/mpm/winnt mpm_winnt.c
> >   Log:
> >     When restarting [always graceful on Win32], we don't repeat pre_mpm
> >     (Unix doesn't, we shouldn't either.)  [Ryan Bloom]
> >    
> >   -    if ((parent_pid == my_pid) || one_process) {
> >   +    /* ### If non-graceful restarts are ever introduced - we need to rerun 
> >   +     * the pre_mpm hook on subsequent non-graceful restarts.  But Win32 
> >   +     * has only graceful style restarts - and we need this hook to act 
> >   +     * the same on Win32 as on Unix.
> >   +     */
> >   +    if (!restart && ((parent_pid == my_pid) || one_process)) {
> >            /* Set up the scoreboard. */
> >            if (ap_run_pre_mpm(pconf, SB_SHARED) != OK) {
> >                return 1;
> 
> While I agree with this patch, you also need to kill the cleanup on the
> scoreboard, so that it isn't set to NULL when pconf is cleared.

Actually, the cleanup never runs because it never finds the data in
the pconf pool :)  But yes it needs to be dealt with.

I was cleaning this issue up this morning (removing cleanups, removing
logic I added recently to restore the proper running_generation field)
and realized (or was confused into thinking) that we aren't handling
non-graceful restart anymore, in that we don't start back with a fresh
scoreboard (fresh except for the running_generation field).  Now,
whatever was in the scoreboard from the previous, whacked generation
of children is still there.

For now I've set aside the cleanup changes until there is some
resolution on how it should be handled (should MPM have to clean up,
should scoreboard-create clean up when it realizes that it doesn't
have to reallocate, etc.).

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c

Posted by Ryan Bloom <rb...@covalent.net>.
Cool, thanks for the info.  This is much clearer now.

Ryan

----------------------------------------------
Ryan Bloom                  rbb@covalent.net
645 Howard St.              rbb@apache.org
San Francisco, CA 

> -----Original Message-----
> From: William A. Rowe, Jr. [mailto:wrowe@rowe-clan.net]
> Sent: Wednesday, March 20, 2002 8:28 AM
> To: dev@httpd.apache.org
> Subject: RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
> 
> At 08:41 AM 3/20/2002, you wrote:
> 
> >yeah, but the new child process needs the same information as the old
> >one did.  Won't you need to pass the scoreboard to the new child, and
> >isn't that done by the pre_mpm hook?  I could very easily be wrong
about
> >which hook does that work, but the last time I looked, I thought it
was
> >pre_mpm.
> 
> No, pre_mpm doesn't pass anything from parent->child [that would be
> broken already by the pre_mpm-only-once patch.]
> 
> The create_process logic within mpm_winnt calls two passing fns,
> one of handles in general, one for the listeners.  It's invoked every
> time we create a child.
> 
> We have two matching fn's called by child_main() that slurp those
> general handlers and listeners, respectively.
> 
> Whatever the ap_scoreboard_shm's os_handle, we pass it.  We
> call ap_scoreboard_init, not to init the score, but to init the
> local storage, e.g. array pointers into the scoreboard memory.
> The memory isn't passed to the child, the handle to this memory
> is passed; the child reopens the scoreboard by its handle.
> 
> Bill
> 



RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 08:41 AM 3/20/2002, you wrote:

>yeah, but the new child process needs the same information as the old
>one did.  Won't you need to pass the scoreboard to the new child, and
>isn't that done by the pre_mpm hook?  I could very easily be wrong about
>which hook does that work, but the last time I looked, I thought it was
>pre_mpm.

No, pre_mpm doesn't pass anything from parent->child [that would be
broken already by the pre_mpm-only-once patch.]

The create_process logic within mpm_winnt calls two passing fns,
one of handles in general, one for the listeners.  It's invoked every
time we create a child.

We have two matching fn's called by child_main() that slurp those
general handlers and listeners, respectively.

Whatever the ap_scoreboard_shm's os_handle, we pass it.  We
call ap_scoreboard_init, not to init the score, but to init the
local storage, e.g. array pointers into the scoreboard memory.
The memory isn't passed to the child, the handle to this memory
is passed; the child reopens the scoreboard by its handle.

Bill



RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c

Posted by Ryan Bloom <rb...@covalent.net>.
> >And, you
> >need to find some way to pass the scoreboard back to the child
process
> >because you are about to start a new one.
> 
> What are you talking about, we killed Kenny :0/  This next generation
we
> have new offspring.
> 

yeah, but the new child process needs the same information as the old
one did.  Won't you need to pass the scoreboard to the new child, and
isn't that done by the pre_mpm hook?  I could very easily be wrong about
which hook does that work, but the last time I looked, I thought it was
pre_mpm.

Ryan



Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 11:31 PM 3/19/2002, you wrote:
>On 20 Mar 2002 wrowe@apache.org wrote:
>
> >   +    if (!restart && ((parent_pid == my_pid) || one_process)) {
> >            /* Set up the scoreboard. */
> >            if (ap_run_pre_mpm(pconf, SB_SHARED) != OK) {
> >                return 1;
>
>While I agree with this patch, you also need to kill the cleanup on the
>scoreboard, so that it isn't set to NULL when pconf is cleared.

No you do not kill the cleanup - you rescope it.  See the next patch.
Really, lovely, let's off and leave a dangling object that fell out of scope
with no cleanup.  pre_mpm requires a process->pool.

>And, you
>need to find some way to pass the scoreboard back to the child process
>because you are about to start a new one.

What are you talking about, we killed Kenny :0/  This next generation we
have new offspring.