First experiments show that event does not always call ap_start_lingering_close(c) and exactly for the interesting cases.
As I read it, the reason is that ap_start_lingering_close() does a flush on the connection and event wants to start lingering close when shifting the connection from its keepalive queue to the proper lingering queue. It is not holding a lock, but its a tight loop where it does not like to block. So, event has its own variants of start_lingering_close. I added ap_prep_lingering_close(c) to the API which is then called from everyone and runs the pre_close hook. With that in place, I see proper GOAWAYs in Chrome on a keepalive connection. However if a pre_close hook wants to do more, or if we do care about protocols that need a real conversation on a keep alive shutdown, we need some mechanism to let the protocol indicate, maybe as return from pre_close (making it run_first) that the connection should be transferred back from keepalive to normal processing. It would then be up to the protocol handler to manage the state and all... More complicated and probably overkill for the <32 bytes http/2 likes to send. I just wanted to mention it... Cheers, Stefan > Am 22.01.2016 um 16:13 schrieb Jim Jagielski <[email protected]>: > > +1 > >> On Jan 22, 2016, at 9:17 AM, Stefan Eissing <[email protected]> >> wrote: >> >> >>> Am 22.01.2016 um 15:16 schrieb Eric Covener <[email protected]>: >>> >>> On Fri, Jan 22, 2016 at 9:12 AM, Stefan Eissing >>> <[email protected]> wrote: >>>> With the timeout behaviour of SSL reads fixed, I am making another attempt >>>> at have http/2 connections behave properly in async MPMs, e.g. event. One >>>> thing for that necessary as change in server is the hook to send a last >>>> GOAWAY frame before the connection is closed. Not sending it seems to >>>> confuse browsers, leading to unwanted failure of the subsequent request. >>>> >>>> You opinion therefore is required. On inspection of the connection >>>> shutdown handling, it seems that *all* MPM modules call, directly or >>>> indirectly, ap_start_lingering_close(). Keepalive connections are still >>>> healthy at this point and pool cleanup happens later, so this seems to be >>>> the place to install a new hook: >>>> >>>> AP_DECLARE_HOOK(int,pre_close_connection,(conn_rec *c)) as RUN_ALL >>>> >>>> Does this seem like the right place? Is the name any good? >>>> >>>> (forget the question about naming, should not have disturbed the beast...) >>> >>> Sounds reasonable, and c->pool cleanups you probably found are too >>> late to write anything. >> >> Indeed. All MPMs seem to cleanup pools way afterwards. Which is correct, I >> believe. >
