httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Eissing <stefan.eiss...@greenbytes.de>
Subject Re: svn commit: r1818960 - /httpd/httpd/trunk/server/mpm/event/event.c
Date Fri, 22 Dec 2017 17:14:59 GMT


> Am 22.12.2017 um 00:31 schrieb Yann Ylavic <ylavic.dev@gmail.com>:
> 
> On Thu, Dec 21, 2017 at 7:04 PM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
>> On Thu, Dec 21, 2017 at 6:59 PM, Yann Ylavic <ylavic.dev@gmail.com> wrote:
>>> On Thu, Dec 21, 2017 at 6:49 PM, Stefan Eissing <stefan@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






Mime
View raw message