> Am 14.10.2021 um 11:07 schrieb Ruediger Pluem <rpl...@apache.org>:
>
>
>
> On 10/12/21 3:34 PM, ic...@apache.org wrote:
>> Author: icing
>> Date: Tue Oct 12 13:34:01 2021
>> New Revision: 1894163
>>
>> URL: http://svn.apache.org/viewvc?rev=1894163&view=rev
>> Log:
>> *) mod_http2:
>> - Fixed an issue since 1.15.24 that "Server" headers in proxied requests
>> were overwritten instead of preserved. [PR by @daum3ns]
>> - Added directove 'H2StreamTimeout' to configure a separate value for
>> HTTP/2
>> streams, overriding server's 'Timeout' configuration. [rpluem]
>> - HTTP/2 connections now use pollsets to monitor the status of the
>> ongoing streams and their main connection when host OS allows this.
>> - Removed work-arounds for older versions of libnghttp2 and checking
>> during configure that at least version 1.15.0 is present.
>> - The HTTP/2 connection state handler, based on an experiment and draft
>> at the IETF http working group (abandoned for some time), has been
>> removed.
>> - H2SerializeHeaders no longer has an effect. A warning is logged when
>> it is
>> set to "on". The switch enabled the internal writing of requests to be
>> parsed
>> by the internal HTTP/1.1 protocol handler and was introduced to avoid
>> potential incompatibilities during the introduction of HTTP/2.
>> - Removed the abort/redo of tasks when mood swings lower the active
>> limit.
>>
>>
>> Added:
>> httpd/httpd/trunk/changes-entries/http2_additions.txt
>> httpd/httpd/trunk/modules/http2/h2_c1.c
>> httpd/httpd/trunk/modules/http2/h2_c1.h
>> httpd/httpd/trunk/modules/http2/h2_c1_io.c
>> httpd/httpd/trunk/modules/http2/h2_c1_io.h
>> httpd/httpd/trunk/modules/http2/h2_c2.c
>> httpd/httpd/trunk/modules/http2/h2_c2.h
>> httpd/httpd/trunk/modules/http2/h2_c2_filter.c
>> httpd/httpd/trunk/modules/http2/h2_c2_filter.h
>> httpd/httpd/trunk/modules/http2/h2_conn_ctx.c
>> httpd/httpd/trunk/modules/http2/h2_conn_ctx.h
>> httpd/httpd/trunk/modules/http2/h2_protocol.c
>> httpd/httpd/trunk/modules/http2/h2_protocol.h
>> Removed:
>> httpd/httpd/trunk/modules/http2/h2_alt_svc.c
>> httpd/httpd/trunk/modules/http2/h2_alt_svc.h
>> httpd/httpd/trunk/modules/http2/h2_conn.c
>> httpd/httpd/trunk/modules/http2/h2_conn.h
>> httpd/httpd/trunk/modules/http2/h2_conn_io.c
>> httpd/httpd/trunk/modules/http2/h2_conn_io.h
>> httpd/httpd/trunk/modules/http2/h2_ctx.c
>> httpd/httpd/trunk/modules/http2/h2_ctx.h
>> httpd/httpd/trunk/modules/http2/h2_filter.c
>> httpd/httpd/trunk/modules/http2/h2_filter.h
>> httpd/httpd/trunk/modules/http2/h2_from_h1.c
>> httpd/httpd/trunk/modules/http2/h2_from_h1.h
>> httpd/httpd/trunk/modules/http2/h2_h2.c
>> httpd/httpd/trunk/modules/http2/h2_h2.h
>> httpd/httpd/trunk/modules/http2/h2_task.c
>> httpd/httpd/trunk/modules/http2/h2_task.h
>> Modified:
>> httpd/httpd/trunk/CMakeLists.txt
>> httpd/httpd/trunk/modules/http2/NWGNUmod_http2
>> httpd/httpd/trunk/modules/http2/config2.m4
>> httpd/httpd/trunk/modules/http2/h2.h
>> httpd/httpd/trunk/modules/http2/h2_bucket_beam.c
>> httpd/httpd/trunk/modules/http2/h2_bucket_beam.h
>> httpd/httpd/trunk/modules/http2/h2_config.c
>> httpd/httpd/trunk/modules/http2/h2_config.h
>> httpd/httpd/trunk/modules/http2/h2_headers.c
>> httpd/httpd/trunk/modules/http2/h2_mplx.c
>> httpd/httpd/trunk/modules/http2/h2_mplx.h
>> httpd/httpd/trunk/modules/http2/h2_proxy_session.c
>> httpd/httpd/trunk/modules/http2/h2_proxy_util.c
>> httpd/httpd/trunk/modules/http2/h2_proxy_util.h
>> httpd/httpd/trunk/modules/http2/h2_push.c
>> httpd/httpd/trunk/modules/http2/h2_request.c
>> httpd/httpd/trunk/modules/http2/h2_request.h
>> httpd/httpd/trunk/modules/http2/h2_session.c
>> httpd/httpd/trunk/modules/http2/h2_session.h
>> httpd/httpd/trunk/modules/http2/h2_stream.c
>> httpd/httpd/trunk/modules/http2/h2_stream.h
>> httpd/httpd/trunk/modules/http2/h2_switch.c
>> httpd/httpd/trunk/modules/http2/h2_util.c
>> httpd/httpd/trunk/modules/http2/h2_util.h
>> httpd/httpd/trunk/modules/http2/h2_version.h
>> httpd/httpd/trunk/modules/http2/h2_workers.c
>> httpd/httpd/trunk/modules/http2/h2_workers.h
>> httpd/httpd/trunk/modules/http2/mod_http2.c
>> httpd/httpd/trunk/modules/http2/mod_http2.dsp
>> httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
>> httpd/httpd/trunk/test/modules/http2/test_105_timeout.py
>> httpd/httpd/trunk/test/modules/http2/test_712_buffering.py
>>
>> Modified: httpd/httpd/trunk/CMakeLists.txt
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/CMakeLists.txt?rev=1894163&r1=1894162&r2=1894163&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/CMakeLists.txt (original)
>> +++ httpd/httpd/trunk/CMakeLists.txt Tue Oct 12 13:34:01 2021
>> @@ -469,18 +469,15 @@ SET(mod_http2_extra_defines ssi
>> SET(mod_http2_extra_includes ${NGHTTP2_INCLUDE_DIR})
>> SET(mod_http2_extra_libs ${NGHTTP2_LIBRARIES})
>> SET(mod_http2_extra_sources
>> - modules/http2/h2_alt_svc.c
>> - modules/http2/h2_bucket_eos.c modules/http2/h2_config.c
>> - modules/http2/h2_conn.c modules/http2/h2_conn_io.c
>> - modules/http2/h2_ctx.c modules/http2/h2_filter.c
>> - modules/http2/h2_from_h1.c modules/http2/h2_h2.c
>> - modules/http2/h2_bucket_beam.c
>> - modules/http2/h2_mplx.c modules/http2/h2_push.c
>> - modules/http2/h2_request.c modules/http2/h2_headers.c
>> - modules/http2/h2_session.c modules/http2/h2_stream.c
>> - modules/http2/h2_switch.c
>> - modules/http2/h2_task.c modules/http2/h2_util.c
>> - modules/http2/h2_workers.c
>> + modules/http2/h2_bucket_beam.c modules/http2/h2_bucket_eos.c
>> + modules/http2/h2_c1.c modules/http2/h2_c1_io.c
>> + modules/http2/h2_c2.c modules/http2/h2_c2_filter.c
>> + modules/http2/h2_config.c modules/http2/h2_conn_ctx.c
>> + modules/http2/h2_headers.c modules/http2/h2_mplx.c
>> + modules/http2/h2_protocol.c modules/http2/h2_push.c
>> + modules/http2/h2_request.c modules/http2/h2_session.c
>> + modules/http2/h2_stream.c modules/http2/h2_switch.c
>> + modules/http2/h2_util.c modules/http2/h2_workers.c
>> )
>> SET(mod_ldap_extra_defines LDAP_DECLARE_EXPORT)
>> SET(mod_ldap_extra_libs wldap32)
>>
>> Added: httpd/httpd/trunk/changes-entries/http2_additions.txt
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/changes-entries/http2_additions.txt?rev=1894163&view=auto
>> ==============================================================================
>> --- httpd/httpd/trunk/changes-entries/http2_additions.txt (added)
>> +++ httpd/httpd/trunk/changes-entries/http2_additions.txt Tue Oct 12
>> 13:34:01 2021
>> @@ -0,0 +1,17 @@
>> + *) mod_http2:
>> + - Fixed an issue since 1.15.24 that "Server" headers in proxied
>> requests
>> + were overwritten instead of preserved. [PR by @daum3ns]
>> + - Added directove 'H2StreamTimeout' to configure a separate value for
>> HTTP/2
>> + streams, overriding server's 'Timeout' configuration. [rpluem]
>> + - HTTP/2 connections now use pollsets to monitor the status of the
>> + ongoing streams and their main connection when host OS allows this.
>> + - Removed work-arounds for older versions of libnghttp2 and checking
>> + during configure that at least version 1.15.0 is present.
>> + - The HTTP/2 connection state handler, based on an experiment and draft
>> + at the IETF http working group (abandoned for some time), has been
>> removed.
>> + - H2SerializeHeaders no longer has an effect. A warning is logged when
>> it is
>> + set to "on". The switch enabled the internal writing of requests to
>> be parsed
>> + by the internal HTTP/1.1 protocol handler and was introduced to avoid
>> + potential incompatibilities during the introduction of HTTP/2.
>> + - Removed the abort/redo of tasks when mood swings lower the active
>> limit.
>> + [Ruediger Pluem, daum3ns, Stefan Eissing]
>> \ No newline at end of file
>>
>> Modified: httpd/httpd/trunk/modules/http2/NWGNUmod_http2
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/NWGNUmod_http2?rev=1894163&r1=1894162&r2=1894163&view=diff
>> ==============================================================================
>
>>
>> Modified: httpd/httpd/trunk/modules/http2/h2.h
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2.h?rev=1894163&r1=1894162&r2=1894163&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/http2/h2.h (original)
>> +++ httpd/httpd/trunk/modules/http2/h2.h Tue Oct 12 13:34:01 2021
>
>> @@ -138,8 +152,7 @@ struct h2_request {
>> apr_table_t *headers;
>>
>> apr_time_t request_time;
>> - unsigned int chunked : 1; /* iff request body needs to be forwarded
>> as chunked */
>> - unsigned int serialize : 1; /* iff this request is written in HTTP/1.1
>> serialization */
>> + int chunked; /* iff request body needs to be forwarded
>> as chunked */
>
> Why not leaving it a bitfield although currently only one bit used? Probably
> we have further flags in the future.
I change that.
>> apr_off_t raw_bytes; /* RAW network bytes that generated this
>> request - if known. */
>> int http_status; /* Store a possible HTTP status code that
>> gets
>> * defined before creating the dummy HTTP/1.1
>
>> Modified: httpd/httpd/trunk/modules/http2/h2_bucket_beam.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_bucket_beam.c?rev=1894163&r1=1894162&r2=1894163&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/http2/h2_bucket_beam.c (original)
>> +++ httpd/httpd/trunk/modules/http2/h2_bucket_beam.c Tue Oct 12 13:34:01 2021
>
>> @@ -191,40 +91,53 @@ static apr_bucket *h2_beam_bucket(h2_buc
>> return b;
>> }
>>
>> +static int is_empty(h2_bucket_beam *beam);
>> +static apr_off_t get_buffered_data_len(h2_bucket_beam *beam);
>>
>> -/*******************************************************************************
>> - * bucket beam that can transport buckets across threads
>> -
>> ******************************************************************************/
>> -
>> -static void mutex_leave(apr_thread_mutex_t *lock)
>> +static int h2_blist_count(h2_blist *blist)
>> {
>> - apr_thread_mutex_unlock(lock);
>> -}
>> + apr_bucket *b;
>> + int count = 0;
>>
>> -static apr_status_t mutex_enter(void *ctx, h2_beam_lock *pbl)
>> -{
>> - h2_bucket_beam *beam = ctx;
>> - pbl->mutex = beam->lock;
>> - pbl->leave = mutex_leave;
>> - return apr_thread_mutex_lock(pbl->mutex);
>> + for (b = H2_BLIST_FIRST(blist); b != H2_BLIST_SENTINEL(blist);
>> + b = APR_BUCKET_NEXT(b)) {
>> + ++count;
>> + }
>> + return count;
>> }
>>
>> -static apr_status_t enter_yellow(h2_bucket_beam *beam, h2_beam_lock *pbl)
>> -{
>> - return mutex_enter(beam, pbl);
>> -}
>> +#define H2_BEAM_LOG(beam, c, level, rv, msg, bb) \
>> + do { \
>> + if (APLOG_C_IS_LEVEL((c),(level))) { \
>> + char buffer[4 * 1024]; \
>> + apr_size_t len, bmax = sizeof(buffer)/sizeof(buffer[0]); \
>> + len = bb? h2_util_bb_print(buffer, bmax, "", "", bb) : 0; \
>> + ap_log_cerror(APLOG_MARK, (level), rv, (c), \
>> +
>> "BEAM[%s,%s%sdata=%ld,buckets(send/consumed)=%d/%d]: %s %s", \
>> + (beam)->name, \
>> + (beam)->aborted? "aborted," : "", \
>> + is_empty(beam)? "empty," : "", \
>> + (long)get_buffered_data_len(beam), \
>> + h2_blist_count(&(beam)->buckets_to_send), \
>> + h2_blist_count(&(beam)->buckets_consumed), \
>> + (msg), len? buffer : ""); \
>> + } \
>> + } while (0)
>> +
>>
>> -static void leave_yellow(h2_bucket_beam *beam, h2_beam_lock *pbl)
>> +static int bucket_is_mmap(apr_bucket *b)
>> {
>> - (void)beam;
>> - if (pbl->leave) {
>> - pbl->leave(pbl->mutex);
>> - }
>> +#if APR_HAS_MMAP
>> + return APR_BUCKET_IS_MMAP(b);
>> +#else
>> + /* if it is not defined as enabled, it should always be no */
>> + return 0;
>> +#endif
>> }
>>
>> static apr_off_t bucket_mem_used(apr_bucket *b)
>> {
>> - if (APR_BUCKET_IS_FILE(b)) {
>> + if (APR_BUCKET_IS_FILE(b) || bucket_is_mmap(b)) {
>
> MMaped buckets also consume physical memory once the content was read. Of
> course the pages can be dropped from physical memory
> again if space is needed. Furthermore they consume address space in the
> process, but I admit that this will unlikely cause
> any issues on 64 bit architectures. The only thing that can happen here is
> that people complain about large virtual memory sizes
> of processes which has no real impact in this case.
My (maybe faulty) reasoning here is:
HTTP/2 does not create any FILE or MMAP buckets. It just wants to transfer them
efficiently from c2 to c1. Instead of reading them and copying the chunks, it
apr_mmap_dup() or apr_file_setaside() their content for the receiving bucket
brigade.
This should (as I understand it) not duplicate any memory or use more memory
than in a HTTP/1.1 response (plus the usual h2 overhead).
>
>> return 0;
>> }
>> else {
>> @@ -233,53 +146,37 @@ static apr_off_t bucket_mem_used(apr_buc
>> }
>> }
>>
>> -static int report_consumption(h2_bucket_beam *beam, h2_beam_lock *pbl)
>> +static int report_consumption(h2_bucket_beam *beam, int locked)
>> {
>> int rv = 0;
>> - apr_off_t len = beam->received_bytes - beam->cons_bytes_reported;
>> + apr_off_t len = beam->recv_bytes - beam->recv_bytes_reported;
>> h2_beam_io_callback *cb = beam->cons_io_cb;
>>
>> if (len > 0) {
>> if (cb) {
>> void *ctx = beam->cons_ctx;
>>
>> - if (pbl) leave_yellow(beam, pbl);
>> + if (locked) apr_thread_mutex_unlock(beam->lock);
>> cb(ctx, beam, len);
>> - if (pbl) enter_yellow(beam, pbl);
>> + if (locked) apr_thread_mutex_lock(beam->lock);
>> rv = 1;
>> }
>> - beam->cons_bytes_reported += len;
>> + beam->recv_bytes_reported += len;
>> }
>> return rv;
>> }
>>
>> -static void report_prod_io(h2_bucket_beam *beam, int force, h2_beam_lock
>> *pbl)
>> -{
>> - apr_off_t len = beam->sent_bytes - beam->prod_bytes_reported;
>> - if (force || len > 0) {
>> - h2_beam_io_callback *cb = beam->prod_io_cb;
>> - if (cb) {
>> - void *ctx = beam->prod_ctx;
>> -
>> - leave_yellow(beam, pbl);
>> - cb(ctx, beam, len);
>> - enter_yellow(beam, pbl);
>> - }
>> - beam->prod_bytes_reported += len;
>> - }
>> -}
>> -
>> static apr_size_t calc_buffered(h2_bucket_beam *beam)
>> {
>> apr_size_t len = 0;
>> apr_bucket *b;
>> - for (b = H2_BLIST_FIRST(&beam->send_list);
>> - b != H2_BLIST_SENTINEL(&beam->send_list);
>> + for (b = H2_BLIST_FIRST(&beam->buckets_to_send);
>> + b != H2_BLIST_SENTINEL(&beam->buckets_to_send);
>> b = APR_BUCKET_NEXT(b)) {
>> if (b->length == ((apr_size_t)-1)) {
>> /* do not count */
>> }
>> - else if (APR_BUCKET_IS_FILE(b)) {
>> + else if (APR_BUCKET_IS_FILE(b) || bucket_is_mmap(b)) {
>> /* if unread, has no real mem footprint. */
>
> Same comment as above. No real mem footprint until read, but address space is
> consumed.
>
>> }
>> else {
>
> Regards
>
> Rüdiger
>
