On 24 Jan 2022, at 19:39, Yann Ylavic <ylavic....@gmail.com> wrote:
>>>> --- httpd/httpd/trunk/server/mpm/event/event.c (original)
>>>> +++ httpd/httpd/trunk/server/mpm/event/event.c Fri Jan 21 00:09:24 2022
>>>> @@ -1142,6 +1145,7 @@ read_request:
>>>> */
>>>> if (rc != OK || (cs->pub.state >= CONN_STATE_NUM)
>>>> || (cs->pub.state < CONN_STATE_LINGER
>>>> + && cs->pub.state != CONN_STATE_READ_REQUEST_LINE
>>>> && cs->pub.state != CONN_STATE_WRITE_COMPLETION
>>>> && cs->pub.state !=
>>>> CONN_STATE_CHECK_REQUEST_LINE_READABLE
>>>> && cs->pub.state != CONN_STATE_SUSPENDED)) {
>>>
>>> The issue is that we've never asked process_connection hooks that
>>> don't care about async and co to change cs->pub.state when they are
>>> done, and for instance mod_echo's process_echo_connection() will
>>> consume the whole connection but leave CONN_STATE_READ_REQUEST_LINE as
>>> is, and here we'll no longer turn it to CONN_STATE_LINGER to close the
>>> connection as before hence loop indefinitely between mod_echo and
>>> mpm_event (eating a CPU) for a connection that is EOF already.
>>
>> True - fixed this in r1897423.
>>
>> Instead of assuming that all async MPM’s can handle AGAIN, added a dedicated
>> MPM check for it. This mean no surprises for existing code.
>
> Why? Really I think you are complicating this. Every async MPM should
> be able to handle EAGAIN, i.e. AP_MPMQ_IS_ASYNC == AP_MPM_CAN_AGAIN.
Alas no, for the reason you pointed out above.
There are three MPMs in trunk that can AP_MPMQ_IS_ASYNC, only one of them
understands AGAIN.
Little-Net:httpd-trunk6 minfrin$ grep -r IS_ASYNC server/mpm
server/mpm/motorz/motorz.c: case AP_MPMQ_IS_ASYNC:
server/mpm/simple/simple_api.c: case AP_MPMQ_IS_ASYNC:
server/mpm/event/event.c: case AP_MPMQ_IS_ASYNC:
Remember that the SUSPEND function and the AGAIN function are two completely
separate functions. SUSPEND is all about making sure that some other task can
happen without blocking (proxy, DNS, etc). AGAIN is about making sure the
primary connection itself doesn't block.
Regards,
Graham
—
