On 24 Jan 2022, at 13:56, 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.

This is true.

> That's why I'm proposing above that ssl_hook_process_connection()
> returns SUSPENDED and that we handle it as keep calling
> run_process_connection() after poll()ing here in the MPM. Or have a
> new CONN_STATE_WAIT_IO or something set by the hooks, though since we
> have SUSPENDED already it looks unnecessary.

SUSPENDED already has the opposite meaning, in that a suspended connection will 
be excluded from run_process_connection() until such time as some other event 
has occurred (response on DNS, a proxy connection received something, etc) and 
the connection resumed.

I think we need an AGAIN, like this:

Index: include/httpd.h
===================================================================
--- include/httpd.h     (revision 1897407)
+++ include/httpd.h     (working copy)
@@ -464,6 +464,9 @@
                                  */
 #define SUSPENDED -3 /**< Module will handle the remainder of the request.
                       * The core will never invoke the request again, */
+#define AGAIN -4                /**< Module wants to be called again when
+                                  *  more data is availble.
+                                  */
 
 /** Returned by the bottom-most filter if no data was written.
  *  @see ap_pass_brigade(). */

Regards,
Graham
—

Reply via email to