> Am 22.12.2017 um 00:31 schrieb Yann Ylavic <ylavic....@gmail.com>: > > On Thu, Dec 21, 2017 at 7:04 PM, Yann Ylavic <ylavic....@gmail.com> wrote: >> On Thu, Dec 21, 2017 at 6:59 PM, Yann Ylavic <ylavic....@gmail.com> wrote: >>> On Thu, Dec 21, 2017 at 6:49 PM, Stefan Eissing <ste...@eissing.org> wrote: >>>> Oops, thought it was OK. That causes the hangers? >>> >>> Not anymore :) >>> >>> But h2_conn_run() seems to possibly return CONN_STATE_HANDLER, it >>> looks stange to me, and for once causes hanger ;) >>> What's the meaning of it? >> >> That's for when it's called from h2_protocol_switch() possibly, the >> issue is that it's also called from h2_h2_process_conn(), a >> process_connection hook which returns straight to the MPM... > > Actually I don't see any other possible state than CONN_STATE_LINGER > after h2_conn_run(), the master connection has to be terminated in > both h2_h2_process_conn() hook and > core_upgrade_handler::ap_switch_protocol() cases. > > So how about the attached patch? > It changes h2_conn_run() to finally/unconditionally set > CONN_STATE_LINGER after having flushed and aborted the connection > (aborting prevents further unnecessary actions like > ap_check_pipeline() or lingering before closing the socket, once the > connection is flushed...). > This patch does also s/return DONE/return OK/ where appropriate (the > DONE in core_upgrade_handler() is fine because we want to stop request > processing from there). > <h2_conn_state.patch>
The changes in h2_h2.c and h2_switch.c work find in my test. However, the change in h2_conn.c which sets CONN_STATE_LINGER does not. It prevents the last GOAWAY frame from the server to be sent out, in some tests. The current flow, when the client sends a GOAWAY is that the session transits to H2_SESSION_ST_DONE, the mpm calls the pre_linger hook and that writes the server's GOAWAY (as it does in situations where the connection is shut down after an idle timeout by the mpm itself). mod_http2 also returns from connection procession when the connection can only progress when more frames from the client arrive. For example, when the session is H2_SESSION_ST_DONE, which means that all streams have been processed. This is analog to a request being done on a h1 connection. But the h2session and all its state is still alive. So c->aborted should not be set. And it is definitely not in LINGER. I am all for aligning h2 connection states more with the connection state model that mpm_event has (and vice versa). Maybe we have to lock ourselves into a room with a whiteboard and beers and figure this out one day... Do we have good documentation about the CONN_STATE engine somewhere? -Stefan
