Mailing List Archive

Le 30.09.2021 à 15:18, stefan@eissing.org a écrit :
> Just a FYI: I announced that I had a rewrite of the connection/stream
> handling in the works and would bring that into trunk soon. An update
> on how that is going:
>
> v2.0.0-rc4 is on github now and solves the problem with pipes on our
> beloved Window platform, thanks to user @nono303 for the testing.
>
> I have asked on twitter for some more volunteers on testing that, so
> maybe next week, when nothing more negative comes up, I will bring
> that into trunk.
>
> On a "real" OS, there will then be a apr_pollset for monitoring main
> connections and streams (secondary connections). And those will then
> have a file handle for input polling. This gives opportunities for
> certain proxy modules to use pollsets also, if we want to.
>
> Overall, this allowed the implementation to reduce complexity. The
> naming should now make an easier read. Where applicable "conn_rec *c"
> has been renamed to "c1" for a main connection and "c2" for a
> secondary one and filters/functions have been renamed to show
> which connection type they are intended for.
>
> Beams have been simplified and no longer use "proxy" buckets. MMAP
> buckets are transferred more efficiently.
>
> Rüdiger's "H2StreamTimeout" has been added and test cases gave some
> surprises. Turns out ap_die() behaves different depending on the
> presence of the H1 header filter in the out filter chain. Who knew?
>
> Performance is improved under load. I think its basically less CPU
> usage that raises the request/s ceiling by 5-10% on my macOS machine
> when serving static files.
>
> Cheers,
> Stefan
>

On 9/30/21 3:18 PM, stefan@eissing.org wrote:

>
> Rüdiger's "H2StreamTimeout" has been added and test cases gave some
> surprises. Turns out ap_die() behaves different depending on the
> presence of the H1 header filter in the out filter chain. Who knew?
>

If the H1 header filter is still there it means:

1. I have sent any response yet.
2. I could still sent a response (at least an internal server error) and I should do this.

How can the filter be away when in ap_die()?

1. ap_die happens after the status code and the header were already sent for the request. Then there is nothing to add and to do.
In this case r->sent_bodyct for the top request (unwinding subrequest) is set.
2. ap_die happens after a brigade with an EOC bucket has passed the H1 header filter. In this case r->sent_bodyct is still unset.

Hence r->sent_bodyct cannot be used as a detection if the filter is still in place or not as a replacement for the current check.
I assume that in for HTTP/2 you want to remove the H1 filter?

Maybe the following would fix your problem?

Index: http_request.c
===================================================================
--- http_request.c (revision 1893605)
+++ http_request.c (working copy)
@@ -101,8 +101,12 @@
/*
* If next != NULL then we left the while above because of
* next->frec == ap_http_header_filter
+ *
+ * If we are on a HTTP/2 stream let the master connection decide whether
+ * an error response needs to be send or not and give it an
+ * internal server error.
*/
- if (next) {
+ if (next || r->connection->master) {
if (type != AP_FILTER_ERROR) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01579)
"Invalid response status %i", type);



Regards

Rüdiger

> Am 06.10.2021 um 11:26 schrieb Ruediger Pluem <rpluem@apache.org>:
>
>
>
> On 9/30/21 3:18 PM, stefan@eissing.org wrote:
>
>>
>> Rüdiger's "H2StreamTimeout" has been added and test cases gave some
>> surprises. Turns out ap_die() behaves different depending on the
>> presence of the H1 header filter in the out filter chain. Who knew?
>>
>
> If the H1 header filter is still there it means:
>
> 1. I have sent any response yet.
> 2. I could still sent a response (at least an internal server error) and I should do this.
>
> How can the filter be away when in ap_die()?

H2 removes the filter as it would need to parse it again.

> 1. ap_die happens after the status code and the header were already sent for the request. Then there is nothing to add and to do.
> In this case r->sent_bodyct for the top request (unwinding subrequest) is set.
> 2. ap_die happens after a brigade with an EOC bucket has passed the H1 header filter. In this case r->sent_bodyct is still unset.
>
> Hence r->sent_bodyct cannot be used as a detection if the filter is still in place or not as a replacement for the current check.
> I assume that in for HTTP/2 you want to remove the H1 filter?

Yes. This is the ongoing need to separate "HTTP" from "HTTP/1.1" in our server.

> Maybe the following would fix your problem?

Maybe. But I think we should not code for (c->master == NULL) -> (protocol == http/1.1) and vice versa.

The better way is to:

- change the H1 filter to do its generic HTTP checks and make a meta HEADERS bucket,
that contains status code/description and table, instead of serializing http/1.1
- have a real HTTP1 output filter acts on HEADERS buckets, does the http/1.1
checks and serializes down to the network filters.

HTTP/2 would remove the second filter and keep the first. It would make its own
serialization of HEADERS buckets into H2 HEADER frames. (It basically operates like
this now already, only is H2HEADER buckets are internal.)

The other unification this would allow is to simplify handling of 1xx interim responses and footers.

Protocol output filters would see:

[HEADERS 1xx]* [HEADERS >= 200] [DATA]* [HEADERS (footer)]? [EOS] [EOR]

Cheers,
Stefan

>
> Index: http_request.c
> ===================================================================
> --- http_request.c (revision 1893605)
> +++ http_request.c (working copy)
> @@ -101,8 +101,12 @@
> /*
> * If next != NULL then we left the while above because of
> * next->frec == ap_http_header_filter
> + *
> + * If we are on a HTTP/2 stream let the master connection decide whether
> + * an error response needs to be send or not and give it an
> + * internal server error.
> */
> - if (next) {
> + if (next || r->connection->master) {
> if (type != AP_FILTER_ERROR) {
> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01579)
> "Invalid response status %i", type);
>
>
>
> Regards
>
> Rüdiger