On 24 Jan 2022, at 13:56, Yann Ylavic <[email protected]> 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
—
