Re: httpd 2.4.25, mpm_event, ssl: segfaults

[Yann Ylavic]

On Thu, Feb 16, 2017 at 12:31 AM, Yann Ylavic  wrote:
>
> Actually this is 16K (the maximum size of a TLS record)

... these are the outputs (records) splitted/produced by SSL_write()
when given inputs (plain text) greater than 16K (at once).

Re: httpd 2.4.25, mpm_event, ssl: segfaults

[Yann Ylavic]

On Thu, Feb 16, 2017 at 12:06 AM, Jacob Champion  wrote:
> On 02/15/2017 02:03 PM, Yann Ylavic wrote:
>
>> Assuming so :) there is also the fact that mod_ssl will encrypt/pass
>> 8K buckets at a time, while the core output filter tries to send the
>> whole mmap()ed file, keeping what remains after EAGAIN for the next
>> call (if any).
>
> Oh, right... we split on APR_MMAP_LIMIT in the mmap() case. Which is a nice
> big 4MiB instead of 8KiB. Could it really be as easy as tuning the default
> file bucket size up?

Actually this is 16K (the maximum size of a TLS record), so 16K
buckets are passed to the core output filter (which itself bufferizes
buckets lower than THRESHOLD_MIN_WRITE=4K, up to
THRESHOLD_MAX_BUFFER=64K before sending.

Without SSL, sending is done up to the size of the mmapp()ed file
still, which may make a difference.

Maybe you could try to play with
THRESHOLD_MIN_WRITE/THRESHOLD_MAX_BUFFER in server/core_filters.c for
the SSL case (e.g. MIN=4M and MAX=16M), but that'd still cost some
transient to heap buckets (copies) which don't happen in the non-SSL
case...
>
> --Jacob

Re: httpd 2.4.25, mpm_event, ssl: segfaults

[Jacob Champion]

On 02/15/2017 02:03 PM, Yann Ylavic wrote:

On Wed, Feb 15, 2017 at 9:50 PM, Jacob Champion  wrote:


For the next step, I want to find out why TLS connections see such a big
performance hit when I switch off mmap(), but unencrypted connections
don't... it's such a huge difference that I feel like I must be missing
something obvious.


First, you did "EnableSendfile off", right?


Yep :) But thanks for the reminder anyway; I would have kicked myself...

Also, I have to retract an earlier claim I made: I am now seeing a 
difference between the performance of mmap'd and non-mmap'd responses 
for regular HTTP connections. I don't know if I actually changed 
something, or if the original lack of difference was tester error on my 
part.



Assuming so :) there is also the fact that mod_ssl will encrypt/pass
8K buckets at a time, while the core output filter tries to send the
whole mmap()ed file, keeping what remains after EAGAIN for the next
call (if any).


Oh, right... we split on APR_MMAP_LIMIT in the mmap() case. Which is a 
nice big 4MiB instead of 8KiB. Could it really be as easy as tuning the 
default file bucket size up?


--Jacob

Re: httpd 2.4.25, mpm_event, ssl: segfaults

[Yann Ylavic]

On Wed, Feb 15, 2017 at 9:50 PM, Jacob Champion  wrote:
>
> For the next step, I want to find out why TLS connections see such a big
> performance hit when I switch off mmap(), but unencrypted connections
> don't... it's such a huge difference that I feel like I must be missing
> something obvious.

First, you did "EnableSendfile off", right?

Assuming so :) there is also the fact that mod_ssl will encrypt/pass
8K buckets at a time, while the core output filter tries to send the
whole mmap()ed file, keeping what remains after EAGAIN for the next
call (if any).
That's I think a big difference too, especially on localhost or a
fast/large bandwidth network.


Regards,
Yann.

Re: httpd 2.4.25, mpm_event, ssl: segfaults

[Jacob Champion]

On 02/07/2017 02:32 AM, Niklas Edmundsson wrote:

O_DIRECT also bypasses any read-ahead logic, so you'll have to do nice
and big IO etc to get good performance.


Yep, confirmed... my naive approach to O_DIRECT, which reads from the 
file in the 8K chunks we're used to from the file bucket brigade, 
absolutely mutilates our performance (80% slowdown) *and* rails the disk 
during the load test. Not good.


(I was hoping that combining the O_DIRECT approach with in-memory 
caching would give us the best of both worlds. Nope. A plain read() with 
no explicit caching at all is much, much faster on my machine.)



We've played around with O_DIRECT to optimize the caching process in our
large-file caching module (our backing store is nfs). However, since all
our hosts are running Linux we had much better results with doing plain
reads and utilizing posix_fadvise with POSIX_FADV_WILLNEED to trigger
read-ahead and POSIX_FADV_DONTNEED to drop the original file from cache
when read (as future requests will be served from local disk cache).
We're doing 8MB fadvise chunks to get full streaming performance when
caching large files.


Hmm, I will keep the file advisory API in the back of my head, thanks 
for that.


For the next step, I want to find out why TLS connections see such a big 
performance hit when I switch off mmap(), but unencrypted connections 
don't... it's such a huge difference that I feel like I must be missing 
something obvious.


--Jacob

Re: release v1.9.0

[Stefan Priebe - Profihost AG]

Hi,

still no segfaults.

@Yann
Are those patches (the addon on top of v7) and the one on top of mod_ssl
still correct / needed?

Stefan

Am 15.02.2017 um 12:45 schrieb Stefan Priebe - Profihost AG:
> Hi,
> Am 15.02.2017 um 12:19 schrieb Yann Ylavic:
>> Hi Stefan,
>>
>> On Wed, Feb 15, 2017 at 9:34 AM, Stefan Priebe - Profihost AG
>>  wrote:
>>> Current status: no segfaults.
>>
>> Is this with or without the mpm_event's wakeup and/or allocator patches?
> 
> it's with the mpm_event_listener_wakeup_bug57399_V7 +
> 
> --- a/build/httpd/server/mpm/event/event.c  (revision 1776076)
> +++ b/build/httpd/server/mpm/event/event.c  (working copy)
> @@ -1743,6 +1743,8 @@ static void * APR_THREAD_FUNC listener_thread(apr_
>  enable_listensocks(process_slot);
>  }
>  if (!listeners_disabled) {
> +apr_thread_mutex_t *mutex;
> +
>  lr = (ap_listen_rec *) pt->baton;
>  ap_pop_pool(, worker_queue_info);
> 
> @@ -1751,19 +1753,24 @@ static void * APR_THREAD_FUNC listener_thread(apr_
>  apr_allocator_t *allocator;
> 
>  apr_allocator_create();
> -apr_allocator_max_free_set(allocator,
> -   ap_max_mem_free);
> -apr_pool_create_ex(, pconf, NULL,
> allocator);
> -apr_allocator_owner_set(allocator, ptrans);
> -if (ptrans == NULL) {
> +apr_allocator_max_free_set(allocator,
> ap_max_mem_free);
> +rc = apr_pool_create_ex(, pconf, NULL,
> +allocator);
> +if (rc != APR_SUCCESS) {
>  ap_log_error(APLOG_MARK, APLOG_CRIT, rc,
>   ap_server_conf, APLOGNO(03097)
>   "Failed to create transaction
> pool");
> +apr_allocator_destroy(allocator);
>  signal_threads(ST_GRACEFUL);
>  return NULL;
>  }
> +apr_allocator_owner_set(allocator, ptrans);
> +apr_pool_tag(ptrans, "transaction");
>  }
> -apr_pool_tag(ptrans, "transaction");
> +apr_thread_mutex_create(,
> APR_THREAD_MUTEX_DEFAULT,
> +ptrans);
> +apr_allocator_mutex_set(apr_pool_allocator_get(ptrans),
> +mutex);
> 
>  get_worker(_idle_worker, 1, _were_busy);
>  rc = lr->accept_func(, lr, ptrans);
> 
> 
> +
> 
> Index: a/build/httpd/modules/ssl/ssl_engine_io.c
> ===
> --- a/build/httpd/modules/ssl/ssl_engine_io.c (revision 1781324)
> +++ b/build/httpd/modules/ssl/ssl_engine_io.c (working copy)
> @@ -138,6 +138,7 @@ static int bio_filter_out_pass(bio_filter_out_ctx_
> 
>  outctx->rc = ap_pass_brigade(outctx->filter_ctx->pOutputFilter->next,
>   outctx->bb);
> +apr_brigade_cleanup(outctx->bb);
>  /* Fail if the connection was reset: */
>  if (outctx->rc == APR_SUCCESS && outctx->c->aborted) {
>  outctx->rc = APR_ECONNRESET;
> @@ -1699,13 +1700,12 @@ static apr_status_t ssl_io_filter_output(ap_filter
>  while (!APR_BRIGADE_EMPTY(bb) && status == APR_SUCCESS) {
>  apr_bucket *bucket = APR_BRIGADE_FIRST(bb);
> 
> -if (APR_BUCKET_IS_METADATA(bucket)) {
> +if (APR_BUCKET_IS_METADATA(bucket) || !filter_ctx->pssl) {
>  /* Pass through metadata buckets untouched.  EOC is
>   * special; terminate the SSL layer first. */
>  if (AP_BUCKET_IS_EOC(bucket)) {
>  ssl_filter_io_shutdown(filter_ctx, f->c, 0);
>  }
> -AP_DEBUG_ASSERT(APR_BRIGADE_EMPTY(outctx->bb));
> 
>  /* Metadata buckets are passed one per brigade; it might
>   * be more efficient (but also more complex) to use
> @@ -1712,11 +1712,10 @@ static apr_status_t ssl_io_filter_output(ap_filter
>   * outctx->bb as a true buffer and interleave these with
>   * data buckets. */
>  APR_BUCKET_REMOVE(bucket);
> -APR_BRIGADE_INSERT_HEAD(outctx->bb, bucket);
> -status = ap_pass_brigade(f->next, outctx->bb);
> -if (status == APR_SUCCESS && f->c->aborted)
> -status = APR_ECONNRESET;
> -apr_brigade_cleanup(outctx->bb);
> +APR_BRIGADE_INSERT_TAIL(outctx->bb, bucket);
> +if (bio_filter_out_pass(outctx) < 0) {
> +status = outctx->rc;
> +}
>  }
>

AW: svn commit: r1707087 - /httpd/httpd/trunk/modules/debugging/mod_bucketeer.c

[Plüm , Rüdiger , Vodafone Group]

> -Ursprüngliche Nachricht-
> Von: Yann Ylavic [mailto:ylavic@gmail.com]
> Gesendet: Mittwoch, 15. Februar 2017 14:08
> An: httpd-dev 
> Betreff: Re: svn commit: r1707087 -
> /httpd/httpd/trunk/modules/debugging/mod_bucketeer.c
> 
> [with the patch]
> 
> On Wed, Feb 15, 2017 at 2:07 PM, Yann Ylavic 
> wrote:
> > On Tue, Feb 14, 2017 at 10:21 PM, Jacob Champion
>  wrote:
> >>
> >
> > , hence the (default_)handler probably returned
> >>
> >>> Admittedly bucketeer_out_filter() is not very nice because it does
> not
> >>> "consume" its given brigade (nor does default_handler() clear it
> >>> afterward), but that shouldn't be an issue since anyway nothing
> should
> >>> use them once the request is destroyed.
> >>>
> >>> Do you have a backtrace of the crash (and/or a breakpoint in
> >>> bucketeer_out_filter() for the test)?
> >>
> >>
> >> Attached.
> >
> > Thanks, it shows the request being destroyed with the EOR bucket.
> > However the brigade containing the EOR is also allocated on r->pool,
> > hence remove_empty_buckets()'s loop crashes (AIUI).
> >
> > Here is the (reverse) backtrace:
> >
> > #17 0x00488070 in ap_process_request_after_handler
> > (r=0x7fa70980d0a0) at modules/http/http_request.c:366
> > #16 0x0043a4d6 in ap_pass_brigade (next=0x7fa70981b7d0,
> > bb=0x7fa7097fe088) at server/util_filter.c:610
> > [...]
> > #8  0x004554ca in ap_request_core_filter (f=0x7fa70980ea78,
> > bb=0x7fa7097fe088) at
> > #7  0x0043a4d6 in ap_pass_brigade (next=0x7fa709821cc0,
> > bb=0x7fa709821c80) at server/util_filter.c:610
> > [...]
> > #2  0x004579c0 in ap_core_output_filter (f=0x7fa7097fdcc8,
> > bb=0x7fa709821c80) at server/core_filters.c:467
> > #1  0x00457b32 in send_brigade_nonblocking (s=0x7fa7098250b0,
> > bb=0x7fa709821c80, bytes_written=0x7fa7097fe040, c=0x7fa709825348) at
> > server/core_filters.c:511
> > #0  0x00457f98 in remove_empty_buckets (bb=0x7fa709821c80) at
> > server/core_filters.c:604
> >
> > See how frame #8 changes "bb" to its own "tmp_bb" (allocated on r-
> >pool).
> > Since ap_request_core_filter() is trunk only (r1706669), it also
> > explains why it does not happen in 2.4.x.
> >
> > Does the attached patch work for you?
> > I don't like it too much (if ever relevent), we could also possibly
> > special case the EOR brigade (looks a bit hacky to me) or create
> > tmp_bb on c->pool (and leak a few bytes per request, like
> > ap_process_request_after_handler() already)...

How about creating it from c->pool and storing it in c->notes for the lifetime
of c?

Regards

Rüdiger

Re: mod_remoteip and mod_http2 combined

[William A Rowe Jr]

On Wed, Feb 15, 2017 at 9:02 AM, Sander Hoentjen  wrote:
>
> mod_remote ip has:
> /* mod_proxy creates outgoing connections - we don't want those */
> if (!remoteip_is_server_port(c->local_addr->port)) {
> return DECLINED;
> }
> I am guessing something similar is needed for h2 connections?

I suspect that the mod_remoteip logic is wrong, that it should be guarding
against any subordinate connections and examining only explicitly configured
ports / origin IPs. the PROXY protocol is not part of the HTTP protocol and
incompatible with it, so the trust list logic isn't directly compatible (this is
clearly explained in the PROXY pseudo-RFC.)

Re: mod_remoteip and mod_http2 combined

[Sander Hoentjen]

On 02/15/2017 12:19 PM, Jordan Gigov wrote:
> On 15 February 2017 at 12:50, Sander Hoentjen  wrote:
>> Hey guys,
>>
>> I am trying to use both mod_remoteip with ProxyProtocol and mod_http2.
>> It looks like mod_http2 gets handed the connection before mod_remoteip,
>> so things don't work as they should. ProxyProtocol should always be
>> handled first, since it is prepended to the stream. Any pointers to
>> where in the code I can look to change things around?
>>
>> --
>> Sander
> Try modules/http2/h2_h2.c line 550 add "mod_remoteip.c" after "mod_ssl.c".
>
> This reminds me since remoteip is being updated, maybe it should also
> specify some before and after mods to avoid.
It seems I was wrong, the issue is that for some reason
remoteip_hook_pre_connection is called twice for a h2 connection:

[Wed Feb 15 14:32:08.048968 2017] [remoteip:debug] [pid 8521]
mod_remoteip.c(1015): [client 192.168.122.1:35136] AH03503:
RemoteIPProxyProtocol: enabled on connection to 192.168.122.249:84
[Wed Feb 15 14:32:08.049082 2017] [remoteip:debug] [pid 8521]
mod_remoteip.c(1406): [client 192.168.122.1:35136] AH03511:
RemoteIPProxyProtocol: received valid PROXY header: 192.168.122.1:35136
[Wed Feb 15 14:32:08.049554 2017] [http2:debug] [pid 8521]
h2_session.c(994): [client 192.168.122.1:35136] AH03200: h2_session(4)
created, max_streams=100, stream_mem=32768, workers_limit=6,
workers_max=1, push_diary(type=1,N=256)
[Wed Feb 15 14:32:08.049579 2017] [http2:debug] [pid 8521]
h2_session.c(1088): [client 192.168.122.1:35136] AH03201: h2_session(4):
start, INITIAL_WINDOW_SIZE=65535, MAX_CONCURRENT_STREAMS=100
[Wed Feb 15 14:32:08.049589 2017] [http2:debug] [pid 8521]
h2_session.c(2067): [client 192.168.122.1:35136] AH03079: h2_session(4):
started on localhost.localdomain:84
[Wed Feb 15 14:32:08.049596 2017] [http2:debug] [pid 8521]
h2_session.c(1721): [client 192.168.122.1:35136] AH03078: h2_session(4):
transit [INIT] -- init --> [BUSY]
[Wed Feb 15 14:32:08.049616 2017] [http2:debug] [pid 8521]
h2_session.c(441): [client 192.168.122.1:35136] AH03066: h2_session(4):
recv FRAME[SETTINGS[length=0, stream=0]], frames=0/0 (r/s)
[Wed Feb 15 14:32:08.049628 2017] [http2:debug] [pid 8521]
h2_stream.c(189): [client 192.168.122.1:35136] AH03082: h2_stream(4-1):
opened
[Wed Feb 15 14:32:08.049655 2017] [http2:debug] [pid 8521]
h2_session.c(441): [client 192.168.122.1:35136] AH03066: h2_session(4):
recv FRAME[HEADERS[length=31, hend=1, stream=1, eos=1]], frames=1/0 (r/s)
[Wed Feb 15 14:32:08.049706 2017] [http2:debug] [pid 8521]
h2_session.c(677): [client 192.168.122.1:35136] AH03068: h2_session(4):
sent FRAME[SETTINGS[length=6, stream=0]], frames=2/0 (r/s)
[Wed Feb 15 14:32:08.049768 2017] [remoteip:debug] [pid 8521]
mod_remoteip.c(1015): [client 192.168.122.1:35136] AH03503:
RemoteIPProxyProtocol: enabled on connection to 192.168.122.249:84
[Wed Feb 15 14:32:08.049810 2017] [remoteip:warn] [pid 8521] [client
192.168.122.1:35136] AH03496: RemoteIPProxyProtocol data is missing, but
required! Aborting request.
[Wed Feb 15 14:32:08.049840 2017] [http2:debug] [pid 8521]
h2_request.c(271): [client 192.168.122.1:35136] AH03367: h2_request:
access_status=400, request_create failed


mod_remote ip has:
/* mod_proxy creates outgoing connections - we don't want those */
if (!remoteip_is_server_port(c->local_addr->port)) {
return DECLINED;
}
I am guessing something similar is needed for h2 connections?

-- 
Sander

Re: svn commit: r1707087 - /httpd/httpd/trunk/modules/debugging/mod_bucketeer.c

[Yann Ylavic]

[with the patch]

On Wed, Feb 15, 2017 at 2:07 PM, Yann Ylavic  wrote:
> On Tue, Feb 14, 2017 at 10:21 PM, Jacob Champion  wrote:
>>
>
> , hence the (default_)handler probably returned
>>
>>> Admittedly bucketeer_out_filter() is not very nice because it does not
>>> "consume" its given brigade (nor does default_handler() clear it
>>> afterward), but that shouldn't be an issue since anyway nothing should
>>> use them once the request is destroyed.
>>>
>>> Do you have a backtrace of the crash (and/or a breakpoint in
>>> bucketeer_out_filter() for the test)?
>>
>>
>> Attached.
>
> Thanks, it shows the request being destroyed with the EOR bucket.
> However the brigade containing the EOR is also allocated on r->pool,
> hence remove_empty_buckets()'s loop crashes (AIUI).
>
> Here is the (reverse) backtrace:
>
> #17 0x00488070 in ap_process_request_after_handler
> (r=0x7fa70980d0a0) at modules/http/http_request.c:366
> #16 0x0043a4d6 in ap_pass_brigade (next=0x7fa70981b7d0,
> bb=0x7fa7097fe088) at server/util_filter.c:610
> [...]
> #8  0x004554ca in ap_request_core_filter (f=0x7fa70980ea78,
> bb=0x7fa7097fe088) at
> #7  0x0043a4d6 in ap_pass_brigade (next=0x7fa709821cc0,
> bb=0x7fa709821c80) at server/util_filter.c:610
> [...]
> #2  0x004579c0 in ap_core_output_filter (f=0x7fa7097fdcc8,
> bb=0x7fa709821c80) at server/core_filters.c:467
> #1  0x00457b32 in send_brigade_nonblocking (s=0x7fa7098250b0,
> bb=0x7fa709821c80, bytes_written=0x7fa7097fe040, c=0x7fa709825348) at
> server/core_filters.c:511
> #0  0x00457f98 in remove_empty_buckets (bb=0x7fa709821c80) at
> server/core_filters.c:604
>
> See how frame #8 changes "bb" to its own "tmp_bb" (allocated on r->pool).
> Since ap_request_core_filter() is trunk only (r1706669), it also
> explains why it does not happen in 2.4.x.
>
> Does the attached patch work for you?
> I don't like it too much (if ever relevent), we could also possibly
> special case the EOR brigade (looks a bit hacky to me) or create
> tmp_bb on c->pool (and leak a few bytes per request, like
> ap_process_request_after_handler() already)...
>
> Ideally we'd have c->tmp_bb...
>
> Graham/others, a better idea?
Index: server/request.c
===
--- server/request.c	(revision 1783088)
+++ server/request.c	(working copy)
@@ -2056,12 +2056,13 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_co
 if (!tmp_bb) {
 tmp_bb = f->ctx = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
 }
+APR_BRIGADE_CONCAT(tmp_bb, bb);
 
 /* Reinstate any buffered content */
-ap_filter_reinstate_brigade(f, bb, _upto);
+ap_filter_reinstate_brigade(f, tmp_bb, _upto);
 
-while (!APR_BRIGADE_EMPTY(bb)) {
-apr_bucket *bucket = APR_BRIGADE_FIRST(bb);
+while (!APR_BRIGADE_EMPTY(tmp_bb)) {
+apr_bucket *bucket = APR_BRIGADE_FIRST(tmp_bb);
 
 /* if the core has set aside data, back off and try later */
 if (!flush_upto) {
@@ -2091,9 +2092,9 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_co
 
 /* pass each bucket down the chain */
 APR_BUCKET_REMOVE(bucket);
-APR_BRIGADE_INSERT_TAIL(tmp_bb, bucket);
+APR_BRIGADE_INSERT_TAIL(bb, bucket);
 
-status = ap_pass_brigade(f->next, tmp_bb);
+status = ap_pass_brigade(f->next, bb);
 if (!APR_STATUS_IS_EOF(status) && (status != APR_SUCCESS)) {
 return status;
 }
@@ -2100,7 +2101,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_co
 
 }
 
-ap_filter_setaside_brigade(f, bb);
+ap_filter_setaside_brigade(f, tmp_bb);
 return status;
 }
 

Re: svn commit: r1707087 - /httpd/httpd/trunk/modules/debugging/mod_bucketeer.c

[Yann Ylavic]

On Tue, Feb 14, 2017 at 10:21 PM, Jacob Champion  wrote:
>

, hence the (default_)handler probably returned
>
>> Admittedly bucketeer_out_filter() is not very nice because it does not
>> "consume" its given brigade (nor does default_handler() clear it
>> afterward), but that shouldn't be an issue since anyway nothing should
>> use them once the request is destroyed.
>>
>> Do you have a backtrace of the crash (and/or a breakpoint in
>> bucketeer_out_filter() for the test)?
>
>
> Attached.

Thanks, it shows the request being destroyed with the EOR bucket.
However the brigade containing the EOR is also allocated on r->pool,
hence remove_empty_buckets()'s loop crashes (AIUI).

Here is the (reverse) backtrace:

#17 0x00488070 in ap_process_request_after_handler
(r=0x7fa70980d0a0) at modules/http/http_request.c:366
#16 0x0043a4d6 in ap_pass_brigade (next=0x7fa70981b7d0,
bb=0x7fa7097fe088) at server/util_filter.c:610
[...]
#8  0x004554ca in ap_request_core_filter (f=0x7fa70980ea78,
bb=0x7fa7097fe088) at
#7  0x0043a4d6 in ap_pass_brigade (next=0x7fa709821cc0,
bb=0x7fa709821c80) at server/util_filter.c:610
[...]
#2  0x004579c0 in ap_core_output_filter (f=0x7fa7097fdcc8,
bb=0x7fa709821c80) at server/core_filters.c:467
#1  0x00457b32 in send_brigade_nonblocking (s=0x7fa7098250b0,
bb=0x7fa709821c80, bytes_written=0x7fa7097fe040, c=0x7fa709825348) at
server/core_filters.c:511
#0  0x00457f98 in remove_empty_buckets (bb=0x7fa709821c80) at
server/core_filters.c:604

See how frame #8 changes "bb" to its own "tmp_bb" (allocated on r->pool).
Since ap_request_core_filter() is trunk only (r1706669), it also
explains why it does not happen in 2.4.x.

Does the attached patch work for you?
I don't like it too much (if ever relevent), we could also possibly
special case the EOR brigade (looks a bit hacky to me) or create
tmp_bb on c->pool (and leak a few bytes per request, like
ap_process_request_after_handler() already)...

Ideally we'd have c->tmp_bb...

Graham/others, a better idea?

Re: svn commit: r1782875 [1/3] - in /httpd/httpd/trunk: ./ modules/http2/

[Stefan Eissing]

> Am 15.02.2017 um 13:07 schrieb Yann Ylavic :
> 
>> On Mon, Feb 13, 2017 at 10:00 PM,   wrote:
>> Author: icing
>> Date: Mon Feb 13 21:00:30 2017
>> New Revision: 1782875
>> 
>> URL: http://svn.apache.org/viewvc?rev=1782875=rev
>> Log:
>> On the trunk:
>> 
>> mod_http2: rework of stream states and cleanup handling.
> 
> Nice work Stefan!

Thanks a lot! Appreciate it!

Re: svn commit: r1782875 [1/3] - in /httpd/httpd/trunk: ./ modules/http2/

[Yann Ylavic]

On Mon, Feb 13, 2017 at 10:00 PM,   wrote:
> Author: icing
> Date: Mon Feb 13 21:00:30 2017
> New Revision: 1782875
>
> URL: http://svn.apache.org/viewvc?rev=1782875=rev
> Log:
> On the trunk:
>
> mod_http2: rework of stream states and cleanup handling.

Nice work Stefan!

Re: release v1.9.0

[Stefan Priebe - Profihost AG]

Hi,
Am 15.02.2017 um 12:19 schrieb Yann Ylavic:
> Hi Stefan,
> 
> On Wed, Feb 15, 2017 at 9:34 AM, Stefan Priebe - Profihost AG
>  wrote:
>> Current status: no segfaults.
> 
> Is this with or without the mpm_event's wakeup and/or allocator patches?

it's with the mpm_event_listener_wakeup_bug57399_V7 +

--- a/build/httpd/server/mpm/event/event.c  (revision 1776076)
+++ b/build/httpd/server/mpm/event/event.c  (working copy)
@@ -1743,6 +1743,8 @@ static void * APR_THREAD_FUNC listener_thread(apr_
 enable_listensocks(process_slot);
 }
 if (!listeners_disabled) {
+apr_thread_mutex_t *mutex;
+
 lr = (ap_listen_rec *) pt->baton;
 ap_pop_pool(, worker_queue_info);

@@ -1751,19 +1753,24 @@ static void * APR_THREAD_FUNC listener_thread(apr_
 apr_allocator_t *allocator;

 apr_allocator_create();
-apr_allocator_max_free_set(allocator,
-   ap_max_mem_free);
-apr_pool_create_ex(, pconf, NULL,
allocator);
-apr_allocator_owner_set(allocator, ptrans);
-if (ptrans == NULL) {
+apr_allocator_max_free_set(allocator,
ap_max_mem_free);
+rc = apr_pool_create_ex(, pconf, NULL,
+allocator);
+if (rc != APR_SUCCESS) {
 ap_log_error(APLOG_MARK, APLOG_CRIT, rc,
  ap_server_conf, APLOGNO(03097)
  "Failed to create transaction
pool");
+apr_allocator_destroy(allocator);
 signal_threads(ST_GRACEFUL);
 return NULL;
 }
+apr_allocator_owner_set(allocator, ptrans);
+apr_pool_tag(ptrans, "transaction");
 }
-apr_pool_tag(ptrans, "transaction");
+apr_thread_mutex_create(,
APR_THREAD_MUTEX_DEFAULT,
+ptrans);
+apr_allocator_mutex_set(apr_pool_allocator_get(ptrans),
+mutex);

 get_worker(_idle_worker, 1, _were_busy);
 rc = lr->accept_func(, lr, ptrans);


+

Index: a/build/httpd/modules/ssl/ssl_engine_io.c
===
--- a/build/httpd/modules/ssl/ssl_engine_io.c (revision 1781324)
+++ b/build/httpd/modules/ssl/ssl_engine_io.c (working copy)
@@ -138,6 +138,7 @@ static int bio_filter_out_pass(bio_filter_out_ctx_

 outctx->rc = ap_pass_brigade(outctx->filter_ctx->pOutputFilter->next,
  outctx->bb);
+apr_brigade_cleanup(outctx->bb);
 /* Fail if the connection was reset: */
 if (outctx->rc == APR_SUCCESS && outctx->c->aborted) {
 outctx->rc = APR_ECONNRESET;
@@ -1699,13 +1700,12 @@ static apr_status_t ssl_io_filter_output(ap_filter
 while (!APR_BRIGADE_EMPTY(bb) && status == APR_SUCCESS) {
 apr_bucket *bucket = APR_BRIGADE_FIRST(bb);

-if (APR_BUCKET_IS_METADATA(bucket)) {
+if (APR_BUCKET_IS_METADATA(bucket) || !filter_ctx->pssl) {
 /* Pass through metadata buckets untouched.  EOC is
  * special; terminate the SSL layer first. */
 if (AP_BUCKET_IS_EOC(bucket)) {
 ssl_filter_io_shutdown(filter_ctx, f->c, 0);
 }
-AP_DEBUG_ASSERT(APR_BRIGADE_EMPTY(outctx->bb));

 /* Metadata buckets are passed one per brigade; it might
  * be more efficient (but also more complex) to use
@@ -1712,11 +1712,10 @@ static apr_status_t ssl_io_filter_output(ap_filter
  * outctx->bb as a true buffer and interleave these with
  * data buckets. */
 APR_BUCKET_REMOVE(bucket);
-APR_BRIGADE_INSERT_HEAD(outctx->bb, bucket);
-status = ap_pass_brigade(f->next, outctx->bb);
-if (status == APR_SUCCESS && f->c->aborted)
-status = APR_ECONNRESET;
-apr_brigade_cleanup(outctx->bb);
+APR_BRIGADE_INSERT_TAIL(outctx->bb, bucket);
+if (bio_filter_out_pass(outctx) < 0) {
+status = outctx->rc;
+}
 }
 else {
 /* Filter a data bucket. */


Greets,
Stefan

Re: release v1.9.0

[Yann Ylavic]

Hi Stefan,

On Wed, Feb 15, 2017 at 9:34 AM, Stefan Priebe - Profihost AG
 wrote:
> Current status: no segfaults.

Is this with or without the mpm_event's wakeup and/or allocator patches?

Regards,
Yann.

Re: mod_remoteip and mod_http2 combined

[Jordan Gigov]

Try modules/http2/h2_h2.c line 550 add "mod_remoteip.c" after "mod_ssl.c".

This reminds me since remoteip is being updated, maybe it should also
specify some before and after mods to avoid.

On 15 February 2017 at 12:50, Sander Hoentjen  wrote:
> Hey guys,
>
> I am trying to use both mod_remoteip with ProxyProtocol and mod_http2.
> It looks like mod_http2 gets handed the connection before mod_remoteip,
> so things don't work as they should. ProxyProtocol should always be
> handled first, since it is prepended to the stream. Any pointers to
> where in the code I can look to change things around?
>
> --
> Sander

mod_remoteip and mod_http2 combined

[Sander Hoentjen]

Hey guys,

I am trying to use both mod_remoteip with ProxyProtocol and mod_http2.
It looks like mod_http2 gets handed the connection before mod_remoteip,
so things don't work as they should. ProxyProtocol should always be
handled first, since it is prepended to the stream. Any pointers to
where in the code I can look to change things around?

-- 
Sander

Gitignore for migration

[Jordan Gigov]

While complete migration itself seems to be a few centuries away, I
did work on importing the svn:ignore properties and converting to a
gitignore format.

I'll try attaching it for anyone interested, but if that doesn't work
just ask and I'll paste it directly.


httpd.gitignore
Description: Binary data

Re: release v1.9.0

[Steffen]

Also here windows up and running on AL:

mod_http2 (v1.9.0-git, feats=, nghttp2 1.19.0), initializing...

Steffen


On Wednesday 15/02/2017 at 10:17, Stefan Eissing  wrote:

Thanks.



Am 15.02.2017 um 09:34 schrieb Stefan Priebe - Profihost AG 
:


Current status: no segfaults.



Am 14.02.2017 um 19:59 schrieb Stefan Priebe - Profihost AG:
Hi,

up and running - Thanks!

Greets,
Stefan



Am 14.02.2017 um 17:05 schrieb Stefan Eissing:
Stefan,

if you'd like, please throw
https://github.com/icing/mod_h2/releases/tag/v1.9.0
into your pit of segfaults and let's see if we find a new one!

Many thanks for testing.

Stefan Eissing

bytes GmbH
Hafenstrasse 16
48155 Münster
http://www.greenbytes.de

Re: release v1.9.0

[Stefan Eissing]

Thanks. 

> Am 15.02.2017 um 09:34 schrieb Stefan Priebe - Profihost AG 
> :
> 
> Current status: no segfaults.
> 
>> Am 14.02.2017 um 19:59 schrieb Stefan Priebe - Profihost AG:
>> Hi,
>> 
>> up and running - Thanks!
>> 
>> Greets,
>> Stefan
>> 
>>> Am 14.02.2017 um 17:05 schrieb Stefan Eissing:
>>> Stefan,
>>> 
>>> if you'd like, please throw
>>> https://github.com/icing/mod_h2/releases/tag/v1.9.0
>>> into your pit of segfaults and let's see if we find a new one!
>>> 
>>> Many thanks for testing.
>>> 
>>> Stefan Eissing
>>> 
>>> bytes GmbH
>>> Hafenstrasse 16
>>> 48155 Münster
>>> www.greenbytes.de
>>> 

Re: release v1.9.0

[Stefan Priebe - Profihost AG]

Current status: no segfaults.

Am 14.02.2017 um 19:59 schrieb Stefan Priebe - Profihost AG:
> Hi,
> 
> up and running - Thanks!
> 
> Greets,
> Stefan
> 
> Am 14.02.2017 um 17:05 schrieb Stefan Eissing:
>> Stefan,
>>
>> if you'd like, please throw
>> https://github.com/icing/mod_h2/releases/tag/v1.9.0
>> into your pit of segfaults and let's see if we find a new one!
>>
>> Many thanks for testing.
>>
>> Stefan Eissing
>>
>> bytes GmbH
>> Hafenstrasse 16
>> 48155 Münster
>> www.greenbytes.de
>>