Things have been quite busy here, because of the release. Hence I think it's worth asking again :-).
Regards Rüdiger On 9/14/21 1:43 PM, Ruediger Pluem wrote: > 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 > >
