You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Erik Lotspeich <er...@lotspeich.org> on 2007/11/27 01:25:00 UTC

Please read: Fix for APR bug

Hi all:

I've posted a few messages regarding this topic.  I believe that there 
must be someone interested in this topic who wrote the apr_pools.c 
originally or has worked with apr_pool_note_subprocess().  If not, then I 
suppose that would explain the lack of interest in this problem.

To summarize, I believe that there is a bug either in apr_pools.c (in the 
function free_proc_chain()) or in apr_proc_wait().  I posted a patch to 
this list with a fix that does work.  I didn't begin to dive into 
apr_proc_wait() (which may be the true source of the problem) until I got 
some feedback from the developers on this list.

Any response would be greatly appreciated.

Regards,

Erik.

On Sat, 24 Nov 2007, Erik Lotspeich wrote:

> Hi:
>
> I continued my investigation as to the reason why apr_pool_note_subprocess()
> is not working for me.  It seems that there is a bug in apr_pools.c in the
> function free_proc_chain().  Here's my patch:
>
> --- apr_pools.c 2007-11-24 23:06:12.000000000 -0800
> +++ apr_pools.c+        2007-11-24 23:06:01.000000000 -0800
> @@ -2118,7 +2118,7 @@
> #ifndef NEED_WAITPID
>     /* Pick up all defunct processes */
>     for (pc = procs; pc; pc = pc->next) {
> -        if (apr_proc_wait(pc->proc, NULL, NULL, APR_NOWAIT) !=
> APR_CHILD_NOTDONE)
> +        if (apr_proc_wait(pc->proc, NULL, NULL, APR_NOWAIT) ==
> APR_CHILD_DONE)
>             pc->kill_how = APR_KILL_NEVER;
>     }
> #endif /* !defined(NEED_WAITPID) */
>
> It may be true that apr_proc_wait is the original source of the problem.  In
> theory, both the previous and patched versions of the code above are
> equivalent.  In reality, the doxygen documentation is incomplete.
> apr_proc_wait can return codes other than APR_CHILD_DONE and
> APR_CHILD_NOTDONE.  In my case, the process I to be checked was actually
> running, but an APR_CHILD_NOTDONE code wasn't being set!
>
> Any questions or comments would be appreciated.
>
> Regards,
>
> Erik.
>

Re: Please read: Fix for APR bug

Posted by Erik Lotspeich <er...@lotspeich.org>.
Hi Henry,

Thank you very much for your response.

I believe that you are correct in that an error code would fail to 
generate an APR_CHILD_DONE status while the process may be no longer 
running.  My fix would not be valid in this case.

I will continue to debug the problem with suspicion placed on 
apr_proc_wait as the culprit.  I'll post another patch once I have 
completed this task.

Regards,

Erik.

On Mon, 26 Nov 2007, Henry Jen wrote:

> On Nov 26, 2007 4:25 PM, Erik Lotspeich <er...@lotspeich.org> wrote:
>> Hi all:
>>
>> I've posted a few messages regarding this topic.  I believe that there
>> must be someone interested in this topic who wrote the apr_pools.c
>> originally or has worked with apr_pool_note_subprocess().  If not, then I
>> suppose that would explain the lack of interest in this problem.
>>
>
> It can be hard sometimes to get attention you would like to have,
> especially when others are not experiencing the problem immediately.
> I read the thread, however I did not spend time in this area, so I
> don't think I am qualified to reply.
>
>> To summarize, I believe that there is a bug either in apr_pools.c (in the
>> function free_proc_chain()) or in apr_proc_wait().  I posted a patch to
>> this list with a fix that does work.  I didn't begin to dive into
>> apr_proc_wait() (which may be the true source of the problem) until I got
>> some feedback from the developers on this list.
>>
>
> After a little study, I guess the question is that when the return
> value of apr_proc_wait is not APR_CHILD_NOTDONE, does it guarantee
> there is a subprocess still running? My reading to man page of waitpid
> on Solaris does not indicate that.
>
> So, flip the condition may solve your problem but may cause other issues.
>
> Anyhow, others may know better than I do.
>
> Cheers,
> Henry
>
>> Any response would be greatly appreciated.
>>
>> Regards,
>>
>> Erik.
>>
>> On Sat, 24 Nov 2007, Erik Lotspeich wrote:
>>
>>> Hi:
>>>
>>> I continued my investigation as to the reason why apr_pool_note_subprocess()
>>> is not working for me.  It seems that there is a bug in apr_pools.c in the
>>> function free_proc_chain().  Here's my patch:
>>>
>>> --- apr_pools.c 2007-11-24 23:06:12.000000000 -0800
>>> +++ apr_pools.c+        2007-11-24 23:06:01.000000000 -0800
>>> @@ -2118,7 +2118,7 @@
>>> #ifndef NEED_WAITPID
>>>     /* Pick up all defunct processes */
>>>     for (pc = procs; pc; pc = pc->next) {
>>> -        if (apr_proc_wait(pc->proc, NULL, NULL, APR_NOWAIT) !=
>>> APR_CHILD_NOTDONE)
>>> +        if (apr_proc_wait(pc->proc, NULL, NULL, APR_NOWAIT) ==
>>> APR_CHILD_DONE)
>>>             pc->kill_how = APR_KILL_NEVER;
>>>     }
>>> #endif /* !defined(NEED_WAITPID) */
>>>
>>> It may be true that apr_proc_wait is the original source of the problem.  In
>>> theory, both the previous and patched versions of the code above are
>>> equivalent.  In reality, the doxygen documentation is incomplete.
>>> apr_proc_wait can return codes other than APR_CHILD_DONE and
>>> APR_CHILD_NOTDONE.  In my case, the process I to be checked was actually
>>> running, but an APR_CHILD_NOTDONE code wasn't being set!
>>>
>>> Any questions or comments would be appreciated.
>>>
>>> Regards,
>>>
>>> Erik.
>>>
>>
>

Re: Please read: Fix for APR bug

Posted by Henry Jen <he...@ztune.net>.
On Nov 26, 2007 4:25 PM, Erik Lotspeich <er...@lotspeich.org> wrote:
> Hi all:
>
> I've posted a few messages regarding this topic.  I believe that there
> must be someone interested in this topic who wrote the apr_pools.c
> originally or has worked with apr_pool_note_subprocess().  If not, then I
> suppose that would explain the lack of interest in this problem.
>

It can be hard sometimes to get attention you would like to have,
especially when others are not experiencing the problem immediately.
I read the thread, however I did not spend time in this area, so I
don't think I am qualified to reply.

> To summarize, I believe that there is a bug either in apr_pools.c (in the
> function free_proc_chain()) or in apr_proc_wait().  I posted a patch to
> this list with a fix that does work.  I didn't begin to dive into
> apr_proc_wait() (which may be the true source of the problem) until I got
> some feedback from the developers on this list.
>

After a little study, I guess the question is that when the return
value of apr_proc_wait is not APR_CHILD_NOTDONE, does it guarantee
there is a subprocess still running? My reading to man page of waitpid
on Solaris does not indicate that.

So, flip the condition may solve your problem but may cause other issues.

Anyhow, others may know better than I do.

Cheers,
Henry

> Any response would be greatly appreciated.
>
> Regards,
>
> Erik.
>
> On Sat, 24 Nov 2007, Erik Lotspeich wrote:
>
> > Hi:
> >
> > I continued my investigation as to the reason why apr_pool_note_subprocess()
> > is not working for me.  It seems that there is a bug in apr_pools.c in the
> > function free_proc_chain().  Here's my patch:
> >
> > --- apr_pools.c 2007-11-24 23:06:12.000000000 -0800
> > +++ apr_pools.c+        2007-11-24 23:06:01.000000000 -0800
> > @@ -2118,7 +2118,7 @@
> > #ifndef NEED_WAITPID
> >     /* Pick up all defunct processes */
> >     for (pc = procs; pc; pc = pc->next) {
> > -        if (apr_proc_wait(pc->proc, NULL, NULL, APR_NOWAIT) !=
> > APR_CHILD_NOTDONE)
> > +        if (apr_proc_wait(pc->proc, NULL, NULL, APR_NOWAIT) ==
> > APR_CHILD_DONE)
> >             pc->kill_how = APR_KILL_NEVER;
> >     }
> > #endif /* !defined(NEED_WAITPID) */
> >
> > It may be true that apr_proc_wait is the original source of the problem.  In
> > theory, both the previous and patched versions of the code above are
> > equivalent.  In reality, the doxygen documentation is incomplete.
> > apr_proc_wait can return codes other than APR_CHILD_DONE and
> > APR_CHILD_NOTDONE.  In my case, the process I to be checked was actually
> > running, but an APR_CHILD_NOTDONE code wasn't being set!
> >
> > Any questions or comments would be appreciated.
> >
> > Regards,
> >
> > Erik.
> >
>