On 9/7/21 6:41 PM, Yann Ylavic wrote:
> On Tue, Sep 7, 2021 at 6:08 PM Ruediger Pluem <rpl...@apache.org> wrote:
>>
>> On 9/7/21 11:34 AM, yla...@apache.org wrote:
>>> Author: ylavic
>>> Date: Tue Sep 7 09:34:09 2021
>>> New Revision: 1893014
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1893014&view=rev
>>> Log:
>>> mpm_event: Fix children processes possibly not stopped on graceful restart.
>>>
>>> The number of children spawned can go above active_daemons_limit due to
>>> exponential idle_spawn_rate growth (x 2), enforce the upper limit in
>>> perform_idle_server_maintenance(). PR 63169.
>>>
>>> Proposed by: Joel Self <joelself gmail.com>
>>>
>>> Added:
>>> httpd/httpd/trunk/changes-entries/event_maintenance_spawn_limit.txt
>>> Modified:
>>> httpd/httpd/trunk/server/mpm/event/event.c
>>>
>>> Added: httpd/httpd/trunk/changes-entries/event_maintenance_spawn_limit.txt
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/changes-entries/event_maintenance_spawn_limit.txt?rev=1893014&view=auto
>>> ==============================================================================
>>> --- httpd/httpd/trunk/changes-entries/event_maintenance_spawn_limit.txt
>>> (added)
>>> +++ httpd/httpd/trunk/changes-entries/event_maintenance_spawn_limit.txt Tue
>>> Sep 7 09:34:09 2021
>>> @@ -0,0 +1,2 @@
>>> + *) mpm_event: Fix children processes possibly not stopped on graceful
>>> + restart. PR 63169. [Joel Self <joelself gmail.com>]
>>> \ No newline at end of file
>>>
>>> Modified: httpd/httpd/trunk/server/mpm/event/event.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1893014&r1=1893013&r2=1893014&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
>>> +++ httpd/httpd/trunk/server/mpm/event/event.c Tue Sep 7 09:34:09 2021
>>> @@ -3237,6 +3237,9 @@ static void perform_idle_server_maintena
>>> if (free_length > retained->idle_spawn_rate[child_bucket]) {
>>> free_length = retained->idle_spawn_rate[child_bucket];
>>> }
>>> + if (free_length + active_daemons > active_daemons_limit) {
>>> + free_length = active_daemons_limit - active_daemons;
>>
>> Are we sure that we can't have cases where active_daemons_limit <
>> active_daemons and free_length gets below zero?
>
> If free_length (signed int) gets negative we'll do nothing in the "for
> (i = 0; i < free_length; ...)" loop below, so I think we are safe?
Fair enough. But we could log a strange AH00486 with a negative number of
children we want to spawn.
But for avoiding this it would be sufficient to put
free_length < 0 ? 0 : free_length
as parameter for the ap_log_error call instead of free_length. This would limit
the effort to the case where we need to log.
Regards
Rüdiger
