You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2007/06/02 09:57:01 UTC
Re: svn commit: r543667 - /httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/mpmt_os2.c
On 06/02/2007 01:44 AM, jim@apache.org wrote:
> Author: jim
> Date: Fri Jun 1 16:44:36 2007
> New Revision: 543667
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=543667
> Log:
> Minor nit... be consistent and unset even now :)
>
> Modified:
> httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/mpmt_os2.c
>
> Modified: httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/mpmt_os2.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/mpmt_os2.c?view=diff&rev=543667&r1=543666&r2=543667
> ==============================================================================
> --- httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/mpmt_os2.c (original)
> +++ httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/mpmt_os2.c Fri Jun 1 16:44:36 2007
> @@ -337,6 +337,7 @@
> pid = ap_scoreboard_image->parent[n].pid;
> if (ap_in_pid_table(pid)) {
> kill(pid, is_graceful ? SIGHUP : SIGTERM);
> + ap_unset_pid_table(pid);
Good catch. But is this also correct in the graceful / SIGHUP case?
Couldn't it happen that we want to sent a SIGTERM later?
Regards
Rüdiger
Re: svn commit: r543667 - /httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/mpmt_os2.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 06/02/2007 03:07 PM, Jim Jagielski wrote:
>
> On Jun 2, 2007, at 3:57 AM, Ruediger Pluem wrote:
>
>>
>>
>> On 06/02/2007 01:44 AM, jim@apache.org wrote:
>>
>>> Author: jim
>>> Date: Fri Jun 1 16:44:36 2007
>>> New Revision: 543667
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=543667
>>> Log:
>>> Minor nit... be consistent and unset even now :)
>>>
>>> Modified:
>>> httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/ mpmt_os2.c
>>>
>>> Modified: httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/
>>> mpmt_os2.c
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-pid-
>>> table/server/mpm/mpmt_os2/mpmt_os2.c?
>>> view=diff&rev=543667&r1=543666&r2=543667
>>> =====================================================================
>>> =========
>>> --- httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/
>>> mpmt_os2.c (original)
>>> +++ httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/
>>> mpmt_os2.c Fri Jun 1 16:44:36 2007
>>> @@ -337,6 +337,7 @@
>>> pid = ap_scoreboard_image->parent[n].pid;
>>> if (ap_in_pid_table(pid)) {
>>> kill(pid, is_graceful ? SIGHUP : SIGTERM);
>>> + ap_unset_pid_table(pid);
>>
>>
>> Good catch. But is this also correct in the graceful / SIGHUP case?
>> Couldn't it happen that we want to sent a SIGTERM later?
>>
>
> Could be... It's been a LONG time since I looked at this
> MPM :)
>
BTW: Do we have any OS/2 platform maintainer(s) still around?
If not there may arise the need for us to dump it if there is nobody around
who can maintain it.
Regards
Rüdiger
Re: svn commit: r543667 - /httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/mpmt_os2.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 06/02/2007 03:07 PM, Jim Jagielski wrote:
>
> On Jun 2, 2007, at 3:57 AM, Ruediger Pluem wrote:
>
>>
>>
>> On 06/02/2007 01:44 AM, jim@apache.org wrote:
>>
>>> Author: jim
>>> Date: Fri Jun 1 16:44:36 2007
>>> New Revision: 543667
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=543667
>>> Log:
>>> Minor nit... be consistent and unset even now :)
>>>
>>> Modified:
>>> httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/ mpmt_os2.c
>>>
>>> Modified: httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/
>>> mpmt_os2.c
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-pid-
>>> table/server/mpm/mpmt_os2/mpmt_os2.c?
>>> view=diff&rev=543667&r1=543666&r2=543667
>>> =====================================================================
>>> =========
>>> --- httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/
>>> mpmt_os2.c (original)
>>> +++ httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/
>>> mpmt_os2.c Fri Jun 1 16:44:36 2007
>>> @@ -337,6 +337,7 @@
>>> pid = ap_scoreboard_image->parent[n].pid;
>>> if (ap_in_pid_table(pid)) {
>>> kill(pid, is_graceful ? SIGHUP : SIGTERM);
>>> + ap_unset_pid_table(pid);
>>
>>
>> Good catch. But is this also correct in the graceful / SIGHUP case?
>> Couldn't it happen that we want to sent a SIGTERM later?
>>
>
> Could be... It's been a LONG time since I looked at this
> MPM :)
Digging somewhat deeper into the code I think you are right with your patch and my worries seem
to be wrong. It looks like to me that the OS/2 MPM never shrinks its process pool (like prefork
and worker do) by killing idle processes that are not longer needed. It only seems to start
additional ones if it runs out of enough idle processes. The killing of processes only happens
when the MPM restarts / shuts down and the MPM doesn't seem to care whether the child processes
*really* exit after sending the signal to them (prefork and worker seem to care). So it never deals
with these pids again.
Regards
Rüdiger
Re: svn commit: r543667 - /httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/mpmt_os2.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Jun 2, 2007, at 3:57 AM, Ruediger Pluem wrote:
>
>
> On 06/02/2007 01:44 AM, jim@apache.org wrote:
>> Author: jim
>> Date: Fri Jun 1 16:44:36 2007
>> New Revision: 543667
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=543667
>> Log:
>> Minor nit... be consistent and unset even now :)
>>
>> Modified:
>> httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/
>> mpmt_os2.c
>>
>> Modified: httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/
>> mpmt_os2.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/httpd-pid-
>> table/server/mpm/mpmt_os2/mpmt_os2.c?
>> view=diff&rev=543667&r1=543666&r2=543667
>> =====================================================================
>> =========
>> --- httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/
>> mpmt_os2.c (original)
>> +++ httpd/httpd/branches/httpd-pid-table/server/mpm/mpmt_os2/
>> mpmt_os2.c Fri Jun 1 16:44:36 2007
>> @@ -337,6 +337,7 @@
>> pid = ap_scoreboard_image->parent[n].pid;
>> if (ap_in_pid_table(pid)) {
>> kill(pid, is_graceful ? SIGHUP : SIGTERM);
>> + ap_unset_pid_table(pid);
>
> Good catch. But is this also correct in the graceful / SIGHUP case?
> Couldn't it happen that we want to sent a SIGTERM later?
>
Could be... It's been a LONG time since I looked at this
MPM :)