If I understand your gist correctly, this would allow HTTP/2 processing to return to the main (async) event loop more often. Which would be great.
In the case of HTTP/2, it would be even more cool, to trigger the (re-)processing of an AGAIN connection from another thread. The use case is: H2 main look started request, awaits response HEADERS/DATA *or* incoming data from client. now: timed wait on a condition with read checks on the main connection at dynamic intervals then: return AGAIN (READ mode) to event look, new HEADERS/DATA from request triggers re-process_connection. -Stefan > Am 18.02.2018 um 00:11 schrieb Graham Leggett <[email protected]>: > > Hi all, > > As an extension to the idea of filters being async and being able to yield > and break up their work into small chunks, I am keen to extend that idea to > selected hooks. > > The patch below is a proof of concept, showing what it might look like if the > pre_connection and process_connection hooks were able to return the code > AGAIN. This means “please call me again”. > > In implementing this, we would start at the outermost hook, and then work > inwards. We would also need to change these void functions to return ints, so > that AGAIN could bubble upwards. Eventually the handler hook will be able to > return AGAIN. > > Any module that doesn’t yield just behaves as modules do now. > > Does this make sense? > > Regards, > Graham > -- > > Index: include/http_connection.h > =================================================================== > --- include/http_connection.h (revision 1824594) > +++ include/http_connection.h (working copy) > @@ -105,11 +105,14 @@ > * This hook gives protocol modules an opportunity to set everything up > * before calling the protocol handler. All pre-connection hooks are > * run until one returns something other than ok or decline > + * > + * The pre connection hook may trigger a break in processing and ask to > + * be called again by returning AGAIN. > * @param c The connection on which the request has been received. > * @param csd The mechanism on which this connection is to be read. > * Most times this will be a socket, but it is up to the module > * that accepts the request to determine the exact type. > - * @return OK or DECLINED > + * @return OK, DECLINED or AGAIN > */ > AP_DECLARE_HOOK(int,pre_connection,(conn_rec *c, void *csd)) > > @@ -118,8 +121,11 @@ > * established, the protocol module must read and serve the request. This > * function does that for each protocol module. The first protocol module > * to handle the request is the last module run. > + * > + * Protocol modules may trigger a break in processing and ask to be called > + * again by returning AGAIN. > * @param c The connection on which the request has been received. > - * @return OK or DECLINED > + * @return OK, DECLINED or AGAIN > */ > AP_DECLARE_HOOK(int,process_connection,(conn_rec *c)) > > Index: include/httpd.h > =================================================================== > --- include/httpd.h (revision 1824594) > +++ include/httpd.h (working copy) > @@ -460,6 +460,8 @@ > */ > #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. > + */ > > /** Returned by the bottom-most filter if no data was written. > * @see ap_pass_brigade(). */ > Index: server/connection.c > =================================================================== > --- server/connection.c (revision 1824594) > +++ server/connection.c (working copy) > @@ -29,6 +29,7 @@ > #include "scoreboard.h" > #include "http_log.h" > #include "util_filter.h" > +#include "core.h" > > APR_HOOK_STRUCT( > APR_HOOK_LINK(create_connection) > @@ -207,15 +208,38 @@ > > AP_CORE_DECLARE(void) ap_process_connection(conn_rec *c, void *csd) > { > + conn_config_t *conn_config = ap_get_core_module_config(c->conn_config); > int rc; > + > + switch (AP_CORE_DEFAULT(conn_config, milestone, CONN_MILESTONE_START)) { > + case CONN_MILESTONE_START: > + > ap_update_vhost_given_ip(c); > > + case CONN_MILESTONE_PRE_CONNECTION: > + conn_config->milestone = CONN_MILESTONE_START; > rc = ap_run_pre_connection(c, csd); > + if (rc == AGAIN) { > + conn_config->milestone = CONN_MILESTONE_PRE_CONNECTION; > + return; > + } > + > if (rc != OK && rc != DONE) { > c->aborted = 1; > } > > if (!c->aborted) { > - ap_run_process_connection(c); > + > + case CONN_MILESTONE_PROCESS_CONNECTION: > + conn_config->milestone = CONN_MILESTONE_START; > + rc = ap_run_process_connection(c); > + if (rc == AGAIN) { > + conn_config->milestone = CONN_MILESTONE_PROCESS_CONNECTION; > + return; > + } > + > } > + > + } > } > + > Index: server/core.h > =================================================================== > --- server/core.h (revision 1824635) > +++ server/core.h (working copy) > @@ -25,6 +25,13 @@ > #ifndef CORE_H > #define CORE_H > > +/** The connection milestones in the core */ > +typedef enum { > + CONN_MILESTONE_START, > + CONN_MILESTONE_PRE_CONNECTION, > + CONN_MILESTONE_PROCESS_CONNECTION > +} conn_milestone_e; > + > /** > * @brief A structure to contain connection state information > */ > @@ -31,6 +38,8 @@ > typedef struct conn_config_t { > /** Socket belonging to the connection */ > apr_socket_t *socket; > + /** Connection milestones */ > + conn_milestone_e milestone; > } conn_config_t; > > > Index: server/mpm/event/event.c > =================================================================== > --- server/mpm/event/event.c (revision 1824594) > +++ server/mpm/event/event.c (working copy) > @@ -219,6 +219,14 @@ > */ > static apr_pollset_t *event_pollset; > > +typedef enum event_milestone_e event_milestone_e; > + > +enum event_milestone_e { > + EVENT_MILESTONE_START = 0, > + EVENT_MILESTONE_PRE_CONNECTION, > + EVENT_MILESTONE_PROCESS_CONNECTION > +}; > + > typedef struct event_conn_state_t event_conn_state_t; > > /* > @@ -245,6 +253,8 @@ > * hooks) > */ > int suspended; > + /** current milestone */ > + event_milestone_e milestone; > /** memory pool to allocate from */ > apr_pool_t *p; > /** bucket allocator */ > @@ -996,6 +1006,9 @@ > apr_status_t rv; > int rc = OK; > > + switch (AP_CORE_DEFAULT(cs, milestone, EVENT_MILESTONE_START)) { > + case EVENT_MILESTONE_START: > + > if (cs == NULL) { /* This is a new connection */ > listener_poll_type *pt = apr_pcalloc(p, sizeof(*pt)); > cs = apr_pcalloc(p, sizeof(event_conn_state_t)); > @@ -1028,7 +1041,14 @@ > > ap_update_vhost_given_ip(c); > > + case EVENT_MILESTONE_PRE_CONNECTION: > + cs->milestone = EVENT_MILESTONE_START; > rc = ap_run_pre_connection(c, sock); > + if (rc == AGAIN) { > + cs->milestone = EVENT_MILESTONE_PRE_CONNECTION; > + return; > + } > + > if (rc != OK && rc != DONE) { > ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(00469) > "process_socket: connection aborted"); > @@ -1083,7 +1103,15 @@ > if (clogging) { > apr_atomic_inc32(&clogged_count); > } > + > + case EVENT_MILESTONE_PROCESS_CONNECTION: > + cs->milestone = EVENT_MILESTONE_START; > rc = ap_run_process_connection(c); > + if (rc == AGAIN) { > + cs->milestone = EVENT_MILESTONE_PROCESS_CONNECTION; > + return; > + } > + > if (clogging) { > apr_atomic_dec32(&clogged_count); > } > @@ -1095,6 +1123,7 @@ > } > } > } > + > /* > * The process_connection hooks above should set the connection state > * appropriately upon return, for event MPM to either: > @@ -1245,6 +1274,8 @@ > cs->pub.state == CONN_STATE_LINGER_SHORT)) { > process_lingering_close(cs); > } > + > + } > } > > /* Put a SUSPENDED connection back into a queue. */ > > > Regards, > Graham > — >
