When looking at this again while researching something I couldn't answer myself the questions below and as event mpm and mod_http2 are sometimes pretty complex I thought I ask :-)
On 7/3/19 3:46 PM, ic...@apache.org wrote: > Author: icing > Date: Wed Jul 3 13:46:31 2019 > New Revision: 1862475 > > URL: http://svn.apache.org/viewvc?rev=1862475&view=rev > Log: > *) mod_http2/mpm_event: Fixes the behaviour when a HTTP/2 connection has > nothing > more to write with streams ongoing (flow control block). The timeout > waiting > for the client to send WINODW_UPDATE was incorrectly KeepAliveTimeout > and not > Timeout as it should be. Fixes PR 63534. [Yann Ylavic, Stefan Eissing] > > > Modified: > httpd/httpd/trunk/CHANGES > httpd/httpd/trunk/modules/http2/h2_conn.c > httpd/httpd/trunk/modules/http2/h2_version.h > httpd/httpd/trunk/server/mpm/event/event.c > > Modified: httpd/httpd/trunk/modules/http2/h2_conn.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_conn.c?rev=1862475&r1=1862474&r2=1862475&view=diff > ============================================================================== > --- httpd/httpd/trunk/modules/http2/h2_conn.c (original) > +++ httpd/httpd/trunk/modules/http2/h2_conn.c Wed Jul 3 13:46:31 2019 > @@ -231,6 +231,13 @@ apr_status_t h2_conn_run(conn_rec *c) > case H2_SESSION_ST_BUSY: > case H2_SESSION_ST_WAIT: > c->cs->state = CONN_STATE_WRITE_COMPLETION; > + if (c->cs && (session->open_streams || > !session->remote.emitted_count)) { > + /* let the MPM know that we are not done and want > + * the Timeout behaviour instead of a KeepAliveTimeout > + * See PR 63534. > + */ > + c->cs->sense = CONN_SENSE_WANT_READ; Can we get here with session->open_streams > 0 and all the open streams are only producing output and none of them is waiting for something from the client? > + } > break; > case H2_SESSION_ST_CLEANUP: > case H2_SESSION_ST_DONE: > > Modified: httpd/httpd/trunk/server/mpm/event/event.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1862475&r1=1862474&r2=1862475&view=diff > ============================================================================== > --- httpd/httpd/trunk/server/mpm/event/event.c (original) > +++ httpd/httpd/trunk/server/mpm/event/event.c Wed Jul 3 13:46:31 2019 > @@ -1153,10 +1153,11 @@ read_request: > else if (ap_filter_should_yield(c->output_filters)) { > pending = OK; > } > - if (pending == OK) { > + if (pending == OK || (pending == DECLINED && > + cs->pub.sense == CONN_SENSE_WANT_READ)) { > /* Still in WRITE_COMPLETION_STATE: > - * Set a write timeout for this connection, and let the > - * event thread poll for writeability. > + * Set a read/write timeout for this connection, and let the > + * event thread poll for read/writeability. > */ > cs->queue_timestamp = apr_time_now(); > notify_suspend(cs); Hm. Showing following code lines from trunk for my next question: update_reqevents_from_sense(cs, -1); if cs->pub.sense == CONN_SENSE_WANT_READ we subscribe on to POLLIN only on the pfd apr_thread_mutex_lock(timeout_mutex); TO_QUEUE_APPEND(cs->sc->wc_q, cs); rv = apr_pollset_add(event_pollset, &cs->pfd); If the read event triggers we will get back to the if (cs->pub.state == CONN_STATE_WRITE_COMPLETION) { int pending = DECLINED; above. This means we try flush the output queue if it is not empty and if it is empty after this we would fall through to set cs->pub.state = CONN_STATE_CHECK_REQUEST_LINE_READABLE; and then add the pfd to the pollset again and poll again. This should trigger the read event again and we would process the input. But if I see this correctly we would need to poll twice for getting the read data processed. OTOH if we do not manage to clear the output queue above we would add the pfd to the pollset again but this time for writing and only once all output has been processed we would take care of the reading. Maybe we should take care of both? Regards Rüdiger
