Re: mod_http2 and Frequent wake-ups for mpm_event
Ok, thanks. I did not have the other one in place. > Am 06.02.2017 um 22:24 schrieb Stefan Priebe - Profihost AG > : > >> Am 06.02.2017 um 22:22 schrieb Stefan Priebe - Profihost AG: >> Hi Stefan, >> >> this one does not apply on top of yann's >> mpm_event_listener_wakeup_bug57399_V7.patch patch. > > i've now used his patch to mpm and yours for mod_http2. > > Stefan > >> >> Stefan >>> Am 06.02.2017 um 15:34 schrieb Stefan Eissing: >>> Yes, that was mixing the order. The attached v4 compiles and runs the h2 >>> tests for me without errors. >>> >>> >>> >>> >>> Am 06.02.2017 um 14:43 schrieb Yann Ylavic : On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing wrote: > Currently running some tests. Have crashes on the original patch in my > test suite. Fixed one, hunting for the next... I think it comes from my change that creates slave connections from master->pool (instead of mplx's), because now slave's pool is already destroyed when h2_mplx_release_and_join()->task_destroy()->h2_slave_destroy() is called (hence the crash). I restored your original code in this new (attached) patch. @s.priebe, would you test this one please? >>> >>> Stefan Eissing >>> >>> bytes GmbH >>> Hafenstrasse 16 >>> 48155 Münster >>> www.greenbytes.de >>>
Re: mod_http2 and Frequent wake-ups for mpm_event
Am 06.02.2017 um 22:22 schrieb Stefan Priebe - Profihost AG: > Hi Stefan, > > this one does not apply on top of yann's > mpm_event_listener_wakeup_bug57399_V7.patch patch. i've now used his patch to mpm and yours for mod_http2. Stefan > > Stefan > Am 06.02.2017 um 15:34 schrieb Stefan Eissing: >> Yes, that was mixing the order. The attached v4 compiles and runs the h2 >> tests for me without errors. >> >> >> >> >> >>> Am 06.02.2017 um 14:43 schrieb Yann Ylavic : >>> >>> On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing >>> wrote: Currently running some tests. Have crashes on the original patch in my test suite. Fixed one, hunting for the next... >>> >>> I think it comes from my change that creates slave connections from >>> master->pool (instead of mplx's), because now slave's pool is already >>> destroyed when >>> h2_mplx_release_and_join()->task_destroy()->h2_slave_destroy() >>> is called (hence the crash). >>> >>> I restored your original code in this new (attached) patch. >>> >>> @s.priebe, would you test this one please? >>> >> >> Stefan Eissing >> >> bytes GmbH >> Hafenstrasse 16 >> 48155 Münster >> www.greenbytes.de >>
Re: mod_http2 and Frequent wake-ups for mpm_event
Hi Stefan, this one does not apply on top of yann's mpm_event_listener_wakeup_bug57399_V7.patch patch. Stefan Am 06.02.2017 um 15:34 schrieb Stefan Eissing: > Yes, that was mixing the order. The attached v4 compiles and runs the h2 > tests for me without errors. > > > > > >> Am 06.02.2017 um 14:43 schrieb Yann Ylavic : >> >> On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing >> wrote: >>> Currently running some tests. Have crashes on the original patch in my test >>> suite. Fixed one, hunting for the next... >> >> I think it comes from my change that creates slave connections from >> master->pool (instead of mplx's), because now slave's pool is already >> destroyed when h2_mplx_release_and_join()->task_destroy()->h2_slave_destroy() >> is called (hence the crash). >> >> I restored your original code in this new (attached) patch. >> >> @s.priebe, would you test this one please? >> > > Stefan Eissing > > bytes GmbH > Hafenstrasse 16 > 48155 Münster > www.greenbytes.de >
Re: svn commit: r1781701 - in /httpd/httpd/trunk: docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c
On 02/04/2017 09:14 PM, drugg...@apache.org wrote:
> Author: druggeri
> Date: Sat Feb 4 20:14:18 2017
> New Revision: 1781701
>
> URL: http://svn.apache.org/viewvc?rev=1781701&view=rev
> Log:
> Change tactic for PROXY processing in Optional case
>
> Modified:
> httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml
> httpd/httpd/trunk/modules/metadata/mod_remoteip.c
>
> Modified: httpd/httpd/trunk/modules/metadata/mod_remoteip.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_remoteip.c?rev=1781701&r1=1781700&r2=1781701&view=diff
> ==
> --- httpd/httpd/trunk/modules/metadata/mod_remoteip.c (original)
> +++ httpd/httpd/trunk/modules/metadata/mod_remoteip.c Sat Feb 4 20:14:18 2017
> @@ -1085,28 +1091,53 @@ static apr_status_t remoteip_input_filte
> memcpy(ctx->header + ctx->rcvd, ptr, len);
> ctx->rcvd += len;
>
> -/* Put this bucket into our temporary storage brigade because
> - we may permit a request without a header. In that case, the
> - data in the temporary storage brigade just gets copied
> - to bb_out */
> -APR_BUCKET_REMOVE(b);
> -apr_bucket_setaside(b, f->c->pool);
> -APR_BRIGADE_INSERT_TAIL(ctx->store_bb, b);
> +apr_bucket_delete(b);
> psts = HDR_NEED_MORE;
>
> if (ctx->version == 0) {
> /* reading initial chunk */
> if (ctx->rcvd >= MIN_HDR_LEN) {
> ctx->version = remoteip_determine_version(f->c,
> ctx->header);
> -if (ctx->version < 0) {
> -psts = HDR_MISSING;
> -}
> -else if (ctx->version == 1) {
> -ctx->mode = AP_MODE_GETLINE;
> -ctx->need = sizeof(proxy_v1);
> +
> +/* We've read enough to know that a header is present.
> In peek mode
> + we purge the bb and can decide to step aside or
> switch to
> + non-speculative read to consume the data */
> +if (ctx->peeking) {
> +apr_brigade_destroy(ctx->bb);
apr_brigade_cleanup is better instead of destroy and create afterwards.
> +
> +if (ctx->version < 0) {
> +ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, f->c,
> APLOGNO(03512)
> + "RemoteIPProxyProtocol: PROXY
> header is missing from "
> + "request. Stepping aside.");
> +ctx->bb = NULL;
> +ctx->done = 1;
> +return ap_get_brigade(f->next, bb_out, mode,
> block, readbytes);
> +}
> +else {
> +/* Rest ctx to initial values */
> +ctx->rcvd = 0;
> +ctx->need = MIN_HDR_LEN;
> +ctx->version = 0;
> +ctx->bb = apr_brigade_create(f->c->pool,
> f->c->bucket_alloc);
> +ctx->done = 0;
> +ctx->mode = AP_MODE_READBYTES;
> +ctx->peeking = 0;
> +
> +ap_get_brigade(f->next, ctx->bb, ctx->mode,
> block,
> + ctx->need - ctx->rcvd);
> +}
> }
> -else if (ctx->version == 2) {
> -ctx->need = MIN_V2_HDR_LEN;
> +else {
> +if (ctx->version < 0) {
> +psts = HDR_MISSING;
> +}
> +else if (ctx->version == 1) {
> +ctx->mode = AP_MODE_GETLINE;
> +ctx->need = sizeof(proxy_v1);
> +}
> +else if (ctx->version == 2) {
> +ctx->need = MIN_V2_HDR_LEN;
> +}
> }
> }
> }
Other comments:
If you are reading in SPECULATIVE mode and renter the filter (e.g. MIN_HDR_LEN
bytes were not available and you were
reading non blocking) or if you just do a second ap_get_brigade in the outer
loop, the returned brigade will contain
all the data you had already seen plus potential new data. So you don't need to
tuck it away in ctx->header. You always
need to examine the whole returned brigade for the header and the brigade
should become longer with a second
ap_get_brigade. If not and you are in non blocking mode no new data was
available and you should leave.
Also take care to handle meta buckets correctly. You could probably
Re: Summary: Broken FastCGI with httpd
On 02/05/2017 05:10 AM, Jim Jagielski wrote:
Not seeing anything fpm_main.c...
Line 1144 in my copy (attached). It's an example of an Apache-specific
fixup that only runs if apache_was_here is *false*.
--Jacob
if (!apache_was_here && env_path_translated != NULL && env_redirect_url !=
NULL &&
env_path_translated != script_path_translated &&
strcmp(env_path_translated, script_path_translated) != 0) {
/*
* pretty much apache specific. If we have a redirect_url
* then our script_filename and script_name point to the
* php executable
* we don't want to do this for the new mod_proxy_fcgi approach,
* where redirect_url may also exist but the below will break
* with rewrites to PATH_INFO, hence the !apache_was_here check
*/
script_path_translated = env_path_translated;
/* we correct SCRIPT_NAME now in case we don't have PATH_INFO */
env_script_name = env_redirect_url;
}
Re: httpd 2.4.25, mpm_event, ssl: segfaults
On 02/03/2017 12:30 AM, Niklas Edmundsson wrote: Methinks this makes mmap+ssl a VERY bad combination if the thing SIGBUS:es due to a simple IO error, I'll proceed with disabling mmap and see if that is a viable way to go for our workload... (Pulling from a parallel conversation, with permission) The question has been raised: is our mmap() optimization really giving us the utility we want for the additional instability we pay? Stefan had this to say: On 02/03/2017 08:32 AM, Stefan Eissing wrote: Experimented on my Ubuntu 14.04 image on Parallels on MacOS 10.12, MacBook Pro mid 2012. Loading a 10 MB file 1000 times over 8 connections: h2load -c 8 -t 8 -n 1000 -m1 http://xxx/10mb.file using HTTP/1.1 and HTTP/2 (limit of 1 stream at a time per connection). Plain and with TLS1.2, transfer speeds in GByte/sec from localhost: H1Plain H1SSL H2Plain H2SSL MMAP on4.3 1.53.8 1.3 off 3.5 1.13.8 1.3 HTTP/2 seems rather unaffected, while HTTP/1.1 experiences significant differences. Hmm... and I replied: Am 03.02.2017 um 21:47 schrieb Jacob Champion : Weird. I can't see any difference for plain HTTP/1.1 when just toggling EnableMMAP, even with EnableSendfile off. I *do* see a significant difference for TLS+HTTP/1.1. That doesn't really make sense to me; is there some other optimization kicking in? sendfile blows the mmap optimization out of the water, but naturally it can't kick in for TLS. I would be interested to see if an O_DIRECT-aware file bucket could speed up the TLS side of things without exposing people to mmap instability. I was also interested to see if there was some mmap() flag we were missing that could fix the problem for us. Turns out a few systems (used to?) have one called MAP_COPY. Linus had a few choice words about it: http://yarchive.net/comp/linux/map_copy.html Linus-insult-rant aside, his point applies here too, I think. We're using mmap() as an optimized read(). We should be focusing on how to use read() in an optimized way. And surely read() for modern systems has come a long way since that thread in 2001? Considering the massive amount of caching that's built into the entire HTTP ecosystem already, O_DIRECT *might* be an effective way to do that (in which we give up filesystem optimizations and caching in return for a DMA into userspace). I have a PoC about halfway done, but I need to split my time this week between this and the FCGI stuff I've been neglecting. --Jacob
Re: mod_http2 and Frequent wake-ups for mpm_event
Am 06.02.2017 um 16:06 schrieb Stefan Eissing: > It does, at the end. Traversed the directories in different order it seems. *arg* sorry > >> Am 06.02.2017 um 16:05 schrieb Stefan Priebe - Profihost AG >> : >> >> Am 06.02.2017 um 16:03 schrieb Stefan Eissing: >>> I took Yann's v3 patch and changed a tiny bit in the h2. I just added a var >>> declaration in event.c without which it did not compile for me. >> >> But your patch does not contain the changes to the event mpm. That's why >> i ask. >> >> Stefan >> >>> -Stefan >>> Am 06.02.2017 um 15:41 schrieb Stefan Priebe - Profihost AG : Hi, so i should test the mpm event part from Yann + your http2 part? Stefan Am 06.02.2017 um 15:34 schrieb Stefan Eissing: > Yes, that was mixing the order. The attached v4 compiles and runs the h2 > tests for me without errors. > > > > > >> Am 06.02.2017 um 14:43 schrieb Yann Ylavic : >> >> On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing >> wrote: >>> Currently running some tests. Have crashes on the original patch in my >>> test suite. Fixed one, hunting for the next... >> >> I think it comes from my change that creates slave connections from >> master->pool (instead of mplx's), because now slave's pool is already >> destroyed when >> h2_mplx_release_and_join()->task_destroy()->h2_slave_destroy() >> is called (hence the crash). >> >> I restored your original code in this new (attached) patch. >> >> @s.priebe, would you test this one please? >> > > Stefan Eissing > > bytes GmbH > Hafenstrasse 16 > 48155 Münster > www.greenbytes.de > >>> >>> Stefan Eissing >>> >>> bytes GmbH >>> Hafenstrasse 16 >>> 48155 Münster >>> www.greenbytes.de >>> > > Stefan Eissing > > bytes GmbH > Hafenstrasse 16 > 48155 Münster > www.greenbytes.de >
Re: mod_http2 and Frequent wake-ups for mpm_event
It does, at the end. Traversed the directories in different order it seems. > Am 06.02.2017 um 16:05 schrieb Stefan Priebe - Profihost AG > : > > Am 06.02.2017 um 16:03 schrieb Stefan Eissing: >> I took Yann's v3 patch and changed a tiny bit in the h2. I just added a var >> declaration in event.c without which it did not compile for me. > > But your patch does not contain the changes to the event mpm. That's why > i ask. > > Stefan > >> -Stefan >> >>> Am 06.02.2017 um 15:41 schrieb Stefan Priebe - Profihost AG >>> : >>> >>> Hi, >>> >>> so i should test the mpm event part from Yann + your http2 part? >>> >>> Stefan >>> >>> Am 06.02.2017 um 15:34 schrieb Stefan Eissing: Yes, that was mixing the order. The attached v4 compiles and runs the h2 tests for me without errors. > Am 06.02.2017 um 14:43 schrieb Yann Ylavic : > > On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing > wrote: >> Currently running some tests. Have crashes on the original patch in my >> test suite. Fixed one, hunting for the next... > > I think it comes from my change that creates slave connections from > master->pool (instead of mplx's), because now slave's pool is already > destroyed when > h2_mplx_release_and_join()->task_destroy()->h2_slave_destroy() > is called (hence the crash). > > I restored your original code in this new (attached) patch. > > @s.priebe, would you test this one please? > Stefan Eissing bytes GmbH Hafenstrasse 16 48155 Münster www.greenbytes.de >> >> Stefan Eissing >> >> bytes GmbH >> Hafenstrasse 16 >> 48155 Münster >> www.greenbytes.de >> Stefan Eissing bytes GmbH Hafenstrasse 16 48155 Münster www.greenbytes.de
Re: mod_http2 and Frequent wake-ups for mpm_event
Am 06.02.2017 um 16:03 schrieb Stefan Eissing: > I took Yann's v3 patch and changed a tiny bit in the h2. I just added a var > declaration in event.c without which it did not compile for me. But your patch does not contain the changes to the event mpm. That's why i ask. Stefan > -Stefan > >> Am 06.02.2017 um 15:41 schrieb Stefan Priebe - Profihost AG >> : >> >> Hi, >> >> so i should test the mpm event part from Yann + your http2 part? >> >> Stefan >> >> Am 06.02.2017 um 15:34 schrieb Stefan Eissing: >>> Yes, that was mixing the order. The attached v4 compiles and runs the h2 >>> tests for me without errors. >>> >>> >>> >>> >>> Am 06.02.2017 um 14:43 schrieb Yann Ylavic : On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing wrote: > Currently running some tests. Have crashes on the original patch in my > test suite. Fixed one, hunting for the next... I think it comes from my change that creates slave connections from master->pool (instead of mplx's), because now slave's pool is already destroyed when h2_mplx_release_and_join()->task_destroy()->h2_slave_destroy() is called (hence the crash). I restored your original code in this new (attached) patch. @s.priebe, would you test this one please? >>> >>> Stefan Eissing >>> >>> bytes GmbH >>> Hafenstrasse 16 >>> 48155 Münster >>> www.greenbytes.de >>> > > Stefan Eissing > > bytes GmbH > Hafenstrasse 16 > 48155 Münster > www.greenbytes.de >
Re: mod_http2 and Frequent wake-ups for mpm_event
I took Yann's v3 patch and changed a tiny bit in the h2. I just added a var declaration in event.c without which it did not compile for me. -Stefan > Am 06.02.2017 um 15:41 schrieb Stefan Priebe - Profihost AG > : > > Hi, > > so i should test the mpm event part from Yann + your http2 part? > > Stefan > > Am 06.02.2017 um 15:34 schrieb Stefan Eissing: >> Yes, that was mixing the order. The attached v4 compiles and runs the h2 >> tests for me without errors. >> >> >> >> >> >>> Am 06.02.2017 um 14:43 schrieb Yann Ylavic : >>> >>> On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing >>> wrote: Currently running some tests. Have crashes on the original patch in my test suite. Fixed one, hunting for the next... >>> >>> I think it comes from my change that creates slave connections from >>> master->pool (instead of mplx's), because now slave's pool is already >>> destroyed when >>> h2_mplx_release_and_join()->task_destroy()->h2_slave_destroy() >>> is called (hence the crash). >>> >>> I restored your original code in this new (attached) patch. >>> >>> @s.priebe, would you test this one please? >>> >> >> Stefan Eissing >> >> bytes GmbH >> Hafenstrasse 16 >> 48155 Münster >> www.greenbytes.de >> Stefan Eissing bytes GmbH Hafenstrasse 16 48155 Münster www.greenbytes.de
Re: Configuration of trusted OCSP responder certificates
On 06-02-17 12:09, Yann Ylavic wrote: >> Is there anyone who can help me with this? Anything we can do? > The patch was committed[1], and is being reviewed (possibly proposed > for backport to 2.4.x soon). Great news, I missed that recent change, thanks! Cheers, Thijs signature.asc Description: OpenPGP digital signature
Re: mod_http2 and Frequent wake-ups for mpm_event
Hi, so i should test the mpm event part from Yann + your http2 part? Stefan Am 06.02.2017 um 15:34 schrieb Stefan Eissing: > Yes, that was mixing the order. The attached v4 compiles and runs the h2 > tests for me without errors. > > > > > >> Am 06.02.2017 um 14:43 schrieb Yann Ylavic : >> >> On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing >> wrote: >>> Currently running some tests. Have crashes on the original patch in my test >>> suite. Fixed one, hunting for the next... >> >> I think it comes from my change that creates slave connections from >> master->pool (instead of mplx's), because now slave's pool is already >> destroyed when h2_mplx_release_and_join()->task_destroy()->h2_slave_destroy() >> is called (hence the crash). >> >> I restored your original code in this new (attached) patch. >> >> @s.priebe, would you test this one please? >> > > Stefan Eissing > > bytes GmbH > Hafenstrasse 16 > 48155 Münster > www.greenbytes.de >
Re: mod_http2 and Frequent wake-ups for mpm_event
Yes, that was mixing the order. The attached v4 compiles and runs the h2 tests for me without errors. ptrans_and_slaves_allocator-v4.patch Description: Binary data > Am 06.02.2017 um 14:43 schrieb Yann Ylavic : > > On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing > wrote: >> Currently running some tests. Have crashes on the original patch in my test >> suite. Fixed one, hunting for the next... > > I think it comes from my change that creates slave connections from > master->pool (instead of mplx's), because now slave's pool is already > destroyed when h2_mplx_release_and_join()->task_destroy()->h2_slave_destroy() > is called (hence the crash). > > I restored your original code in this new (attached) patch. > > @s.priebe, would you test this one please? > Stefan Eissing bytes GmbH Hafenstrasse 16 48155 Münster www.greenbytes.de
Re: mod_http2 and Frequent wake-ups for mpm_event
On Mon, Feb 6, 2017 at 2:23 PM, Ruediger Pluem wrote: > > The question how much cycles this spends in GLIBC / kernel. I don't > know. So maybe its not worth the effort. But if its not worth the > effort it is worth documenting why :-) Sure ;)
Re: mod_http2 and Frequent wake-ups for mpm_event
On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing
wrote:
> Currently running some tests. Have crashes on the original patch in my test
> suite. Fixed one, hunting for the next...
I think it comes from my change that creates slave connections from
master->pool (instead of mplx's), because now slave's pool is already
destroyed when h2_mplx_release_and_join()->task_destroy()->h2_slave_destroy()
is called (hence the crash).
I restored your original code in this new (attached) patch.
@s.priebe, would you test this one please?
Index: server/mpm/event/event.c
===
--- server/mpm/event/event.c (revision 1781789)
+++ 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(&ptrans, worker_queue_info);
@@ -1751,19 +1753,24 @@ static void * APR_THREAD_FUNC listener_thread(apr_
apr_allocator_t *allocator;
apr_allocator_create(&allocator);
-apr_allocator_max_free_set(allocator,
- ap_max_mem_free);
-apr_pool_create_ex(&ptrans, 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(&ptrans, 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(&mutex, APR_THREAD_MUTEX_DEFAULT,
+ptrans);
+apr_allocator_mutex_set(apr_pool_allocator_get(ptrans),
+mutex);
get_worker(&have_idle_worker, 1, &workers_were_busy);
rc = lr->accept_func(&csd, lr, ptrans);
Index: modules/http2/h2_conn.c
===
--- modules/http2/h2_conn.c (revision 1781789)
+++ modules/http2/h2_conn.c (working copy)
@@ -26,6 +26,8 @@
#include
#include
+#include "mpm_common.h" /* for ap_max_mem_free */
+
#include "h2_private.h"
#include "h2.h"
#include "h2_config.h"
@@ -250,6 +252,7 @@ apr_status_t h2_conn_pre_close(struct h2_ctx *ctx,
conn_rec *h2_slave_create(conn_rec *master, int slave_id, apr_pool_t *parent)
{
apr_allocator_t *allocator;
+apr_thread_mutex_t *mutex;
apr_pool_t *pool;
conn_rec *c;
void *cfg;
@@ -264,14 +267,18 @@ conn_rec *h2_slave_create(conn_rec *master, int sl
* another thread.
*/
apr_allocator_create(&allocator);
+apr_allocator_max_free_set(allocator, ap_max_mem_free);
apr_pool_create_ex(&pool, parent, NULL, allocator);
+apr_allocator_owner_set(allocator, pool);
apr_pool_tag(pool, "h2_slave_conn");
-apr_allocator_owner_set(allocator, pool);
+apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, pool);
+apr_allocator_mutex_set(allocator, mutex);
c = (conn_rec *) apr_palloc(pool, sizeof(conn_rec));
if (c == NULL) {
ap_log_cerror(APLOG_MARK, APLOG_ERR, APR_ENOMEM, master,
APLOGNO(02913) "h2_task: creating conn");
+apr_pool_destroy(pool);
return NULL;
}
Index: modules/http2/h2_mplx.c
===
--- modules/http2/h2_mplx.c (revision 1781789)
+++ modules/http2/h2_mplx.c (working copy)
@@ -33,6 +33,7 @@
#include "h2_private.h"
#include "h2_bucket_beam.h"
#include "h2_config.h"
+#include "h2_session.h"
#include "h2_conn.h"
#include "h2_ctx.h"
#include "h2_h2.h"
@@ -259,32 +260,27 @@ static void h2_mplx_destroy(h2_mplx *m)
* their HTTP/1 cousins, the separate allocator seems to work better
* than protecting a shared h2_session one with an own lock.
*/
-h2_mplx *h2_mplx_create(conn_rec *c, apr_pool_t *parent,
-const h2_config *conf,
-
Re: mod_http2 and Frequent wake-ups for mpm_event
Currently running some tests. Have crashes on the original patch in my test suite. Fixed one, hunting for the next... > Am 06.02.2017 um 14:23 schrieb Ruediger Pluem : > > > > On 02/06/2017 01:51 PM, Yann Ylavic wrote: >> On Mon, Feb 6, 2017 at 1:42 PM, Yann Ylavic wrote: >>> On Mon, Feb 6, 2017 at 1:29 PM, Ruediger Pluem wrote: On 02/06/2017 11:56 AM, Yann Ylavic wrote: > Hi Stefan, > > On Mon, Feb 6, 2017 at 9:57 AM, Stefan Priebe - Profihost AG > wrote: >> >> your last patch results in multiple crashes every second: > > Sorry about that, the changes in mpm_event were incorrect (the mutex > was cleared with the pool when recycled, hence its pointer was > dangling). > > New patch attached, this time tested with the httpd framework (where > the previous patch segfaulted too). > > Thanks, > Yann. > Hmm, does it make sense performance wise to create the mutex over and over again? Or is this planned to be improved once it is known to fix the crash issue? >>> >>> Yes, I'm thinking of it, but it's not easy because we need a pool to >>> create the mutex. >>> Using ptrans makes it cleared on recycle (hence re-created), and using >>> the parent pool (pconf) requires synchronization. >>> >>> Possibly we could recycle both the pool (or the allocator) and its >>> mutex, but ap_push/pop_pool() wouldn't be lockless anymore... >> >> Not sure it's really worth it either because apr_thread_mutex_create() >> should boil down to "*mutex = PTHREAD_MUTEXT_INIT" on *nix, and >> probably something equivalent for InitializeCriticalSection() on >> windows... >> We probably not spend many cycles here (compared to more synchronization). > > The question how much cycles this spends in GLIBC / kernel. I don't know. So > maybe its not worth > the effort. But if its not worth the effort it is worth documenting why :-) > > > Regards > > Rüdiger Stefan Eissing bytes GmbH Hafenstrasse 16 48155 Münster www.greenbytes.de
Re: mod_http2 and Frequent wake-ups for mpm_event
On 02/06/2017 01:51 PM, Yann Ylavic wrote: > On Mon, Feb 6, 2017 at 1:42 PM, Yann Ylavic wrote: >> On Mon, Feb 6, 2017 at 1:29 PM, Ruediger Pluem wrote: >>> >>> >>> On 02/06/2017 11:56 AM, Yann Ylavic wrote: Hi Stefan, On Mon, Feb 6, 2017 at 9:57 AM, Stefan Priebe - Profihost AG wrote: > > your last patch results in multiple crashes every second: Sorry about that, the changes in mpm_event were incorrect (the mutex was cleared with the pool when recycled, hence its pointer was dangling). New patch attached, this time tested with the httpd framework (where the previous patch segfaulted too). Thanks, Yann. >>> >>> Hmm, does it make sense performance wise to create the mutex over and over >>> again? >>> Or is this planned to be improved once it is known to fix the crash issue? >> >> Yes, I'm thinking of it, but it's not easy because we need a pool to >> create the mutex. >> Using ptrans makes it cleared on recycle (hence re-created), and using >> the parent pool (pconf) requires synchronization. >> >> Possibly we could recycle both the pool (or the allocator) and its >> mutex, but ap_push/pop_pool() wouldn't be lockless anymore... > > Not sure it's really worth it either because apr_thread_mutex_create() > should boil down to "*mutex = PTHREAD_MUTEXT_INIT" on *nix, and > probably something equivalent for InitializeCriticalSection() on > windows... > We probably not spend many cycles here (compared to more synchronization). The question how much cycles this spends in GLIBC / kernel. I don't know. So maybe its not worth the effort. But if its not worth the effort it is worth documenting why :-) Regards Rüdiger
Re: ssl_io_filter_output vs EOC
On 02/06/2017 01:31 PM, Yann Ylavic wrote:
> On Mon, Feb 6, 2017 at 12:53 PM, Plüm, Rüdiger, Vodafone Group
> wrote:
>>
>> IMHO we currently fail after we processed an EOC (no matter if in the
>> same brigade or in a follow up brigade) and we should continue doing
>> so.
>
> We fail in the same brigade thanks to ssl_filter_write() when pssl is
> NULL, but I think that if we wanted to also fail for subsequent
> brigades, we need something like:
>
> Index: modules/ssl/ssl_engine_io.c
> ===
> --- modules/ssl/ssl_engine_io.c(revision 1781582)
> +++ modules/ssl/ssl_engine_io.c(working copy)
> @@ -1777,14 +1775,15 @@ static apr_status_t ssl_io_filter_output(ap_filter
> return APR_ECONNABORTED;
> }
>
> -/* Reinstate any buffered content */
> -ap_filter_reinstate_brigade(f, bb, &flush_upto);
> -
> if (!filter_ctx->pssl) {
> /* ssl_filter_io_shutdown was called */
> -return ap_pass_brigade(f->next, bb);
> +apr_brigade_cleanup(bb);
> +return APR_EGENERAL;
> }
>
> +/* Reinstate any buffered content */
> +ap_filter_reinstate_brigade(f, bb, &flush_upto);
> +
> inctx = (bio_filter_in_ctx_t *)BIO_get_data(filter_ctx->pbioRead);
> outctx = (bio_filter_out_ctx_t *)BIO_get_data(filter_ctx->pbioWrite);
>
Yes you are correct. I missed that. So some sort of inconsistent behavior here
currently.
I still think that we should not pass on any data here. Only metadata seems to
be acceptable.
But this would require more logic.
Regards
Rüdiger
Re: mod_http2 and Frequent wake-ups for mpm_event
On Mon, Feb 6, 2017 at 1:42 PM, Yann Ylavic wrote: > On Mon, Feb 6, 2017 at 1:29 PM, Ruediger Pluem wrote: >> >> >> On 02/06/2017 11:56 AM, Yann Ylavic wrote: >>> Hi Stefan, >>> >>> On Mon, Feb 6, 2017 at 9:57 AM, Stefan Priebe - Profihost AG >>> wrote: your last patch results in multiple crashes every second: >>> >>> Sorry about that, the changes in mpm_event were incorrect (the mutex >>> was cleared with the pool when recycled, hence its pointer was >>> dangling). >>> >>> New patch attached, this time tested with the httpd framework (where >>> the previous patch segfaulted too). >>> >>> Thanks, >>> Yann. >>> >> >> Hmm, does it make sense performance wise to create the mutex over and over >> again? >> Or is this planned to be improved once it is known to fix the crash issue? > > Yes, I'm thinking of it, but it's not easy because we need a pool to > create the mutex. > Using ptrans makes it cleared on recycle (hence re-created), and using > the parent pool (pconf) requires synchronization. > > Possibly we could recycle both the pool (or the allocator) and its > mutex, but ap_push/pop_pool() wouldn't be lockless anymore... Not sure it's really worth it either because apr_thread_mutex_create() should boil down to "*mutex = PTHREAD_MUTEXT_INIT" on *nix, and probably something equivalent for InitializeCriticalSection() on windows... We probably not spend many cycles here (compared to more synchronization).
Re: mod_http2 and Frequent wake-ups for mpm_event
On Mon, Feb 6, 2017 at 1:29 PM, Ruediger Pluem wrote: > > > On 02/06/2017 11:56 AM, Yann Ylavic wrote: >> Hi Stefan, >> >> On Mon, Feb 6, 2017 at 9:57 AM, Stefan Priebe - Profihost AG >> wrote: >>> >>> your last patch results in multiple crashes every second: >> >> Sorry about that, the changes in mpm_event were incorrect (the mutex >> was cleared with the pool when recycled, hence its pointer was >> dangling). >> >> New patch attached, this time tested with the httpd framework (where >> the previous patch segfaulted too). >> >> Thanks, >> Yann. >> > > Hmm, does it make sense performance wise to create the mutex over and over > again? > Or is this planned to be improved once it is known to fix the crash issue? Yes, I'm thinking of it, but it's not easy because we need a pool to create the mutex. Using ptrans makes it cleared on recycle (hence re-created), and using the parent pool (pconf) requires synchronization. Possibly we could recycle both the pool (or the allocator) and its mutex, but ap_push/pop_pool() wouldn't be lockless anymore...
Re: mod_http2 and Frequent wake-ups for mpm_event
On 02/06/2017 11:56 AM, Yann Ylavic wrote: > Hi Stefan, > > On Mon, Feb 6, 2017 at 9:57 AM, Stefan Priebe - Profihost AG > wrote: >> >> your last patch results in multiple crashes every second: > > Sorry about that, the changes in mpm_event were incorrect (the mutex > was cleared with the pool when recycled, hence its pointer was > dangling). > > New patch attached, this time tested with the httpd framework (where > the previous patch segfaulted too). > > Thanks, > Yann. > Hmm, does it make sense performance wise to create the mutex over and over again? Or is this planned to be improved once it is known to fix the crash issue? Regards Rüdiger
Re: ssl_io_filter_output vs EOC
On Mon, Feb 6, 2017 at 12:53 PM, Plüm, Rüdiger, Vodafone Group
wrote:
>
> IMHO we currently fail after we processed an EOC (no matter if in the
> same brigade or in a follow up brigade) and we should continue doing
> so.
We fail in the same brigade thanks to ssl_filter_write() when pssl is
NULL, but I think that if we wanted to also fail for subsequent
brigades, we need something like:
Index: modules/ssl/ssl_engine_io.c
===
--- modules/ssl/ssl_engine_io.c(revision 1781582)
+++ modules/ssl/ssl_engine_io.c(working copy)
@@ -1777,14 +1775,15 @@ static apr_status_t ssl_io_filter_output(ap_filter
return APR_ECONNABORTED;
}
-/* Reinstate any buffered content */
-ap_filter_reinstate_brigade(f, bb, &flush_upto);
-
if (!filter_ctx->pssl) {
/* ssl_filter_io_shutdown was called */
-return ap_pass_brigade(f->next, bb);
+apr_brigade_cleanup(bb);
+return APR_EGENERAL;
}
+/* Reinstate any buffered content */
+ap_filter_reinstate_brigade(f, bb, &flush_upto);
+
inctx = (bio_filter_in_ctx_t *)BIO_get_data(filter_ctx->pbioRead);
outctx = (bio_filter_out_ctx_t *)BIO_get_data(filter_ctx->pbioWrite);
_
Regards,
Yann.
Re: mod_http2 and Frequent wake-ups for mpm_event
Am 06.02.2017 um 11:56 schrieb Yann Ylavic: > Hi Stefan, > > On Mon, Feb 6, 2017 at 9:57 AM, Stefan Priebe - Profihost AG > wrote: >> >> your last patch results in multiple crashes every second: > > Sorry about that, the changes in mpm_event were incorrect (the mutex > was cleared with the pool when recycled, hence its pointer was > dangling). > > New patch attached, this time tested with the httpd framework (where > the previous patch segfaulted too). Thanks but that one had crashed already once again. This time: error.log: *** Error in `/usr/local/apache/bin/httpd': free(): invalid pointer: 0x7f4bdc023dc0 *** Core was generated by `/usr/local/apache/bin/httpd -DFOREGROUND'. Program terminated with signal SIGSEGV, Segmentation fault. #0 allocator_free (node=0x4480447f447e4466, allocator=0x7f4bc40008c0) at memory/unix/apr_pools.c:381 #0 allocator_free (node=0x4480447f447e4466, allocator=0x7f4bc40008c0) at memory/unix/apr_pools.c:381 #1 apr_pool_destroy (pool=0x7f4bc404a178) at memory/unix/apr_pools.c:856 #2 0x55dab5531bff in task_destroy (m=0x7f4c4c00e8f8, task=0x7f4bc404e210, called_from_master=0) at h2_mplx.c:396 #3 0x55dab5532e6b in task_done_iter (ctx=, val=) at h2_mplx.c:1060 #4 0x7f4c8b6965e6 in apr_hash_do ( comp=comp@entry=0x55dab5545140 , rec=rec@entry=0x7f4c6bfe6480, ht=) at tables/apr_hash.c:542 #5 0x55dab5545b1f in h2_ihash_iter (ih=, fn=fn@entry=0x55dab5532e60 , ctx=ctx@entry=0x7f4c4c00e8f8) at h2_util.c:315 #6 0x55dab5533433 in h2_mplx_release_and_join (m=0x7f4c4c00e8f8, wait=0x7f4c4c00e8a0) at h2_mplx.c:615 #7 0x55dab5538ab4 in session_pool_cleanup (data=0x7f4c04005c78) at h2_session.c:827 #8 0x7f4c8b69f48e in run_cleanups (cref=0x7f4c4c00e878) at memory/unix/apr_pools.c:2352 #9 apr_pool_destroy (pool=0x7f4c4c00e808) at memory/unix/apr_pools.c:804 #10 0x7f4c8b69f745 in apr_pool_clear (pool=0x7f4c78058198) at memory/unix/apr_pools.c:769 #11 0x55dab5570668 in ap_push_pool (queue_info=0x7f4bc40008c0, pool_to_recycle=0xc4041f91) at fdqueue.c:234 #12 0x55dab556b99a in process_lingering_close (cs=0x7f4c78058478, pfd=0x55dab6cf3fa8) at event.c:1513 #13 0x55dab556f4d0 in listener_thread (thd=0x7f4bc40008c0, dummy=0x547dbb6874f83) at event.c:1837 #14 0x7f4c8b46e0a4 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 #15 0x7f4c8b1a362d in clone () from /lib/x86_64-linux-gnu/libc.so.6 Greets, Stefan
AW: ssl_io_filter_output vs EOC
> -Ursprüngliche Nachricht- > Von: Yann Ylavic [mailto:ylavic@gmail.com] > Gesendet: Freitag, 3. Februar 2017 18:06 > An: httpd-dev > Betreff: ssl_io_filter_output vs EOC > > Besides metadata buckets, should the EOC semantic really apply to the > ssl output filter? > > It's not really an issue by now because we never send "data" buckets > after an EOC, but should this happen do we really want to send them > out in clear, after the TLS close notify? > > I'd rather be safe here and return an error up the filters' stack. > Actually this is what is done already if the data come in the same > brigade as the EOC (ssl_filter_write() in the same loop will fail when > the TLS connection is shutdown), but for subsequent brigade(s) we'd > pass through (per EOC semantic). > > This at least requires a consistency "fix" (for the theory, I can't > think of any pratical/reasonable use of data buckets after EOC, e.g. > an Upgrade from TLS to clear looks really hazardous...). > > Is it unacceptable to add an exception to the EOC semantic (for data) > here and fail (as sanity check)? IMHO we currently fail after we processed an EOC (no matter if in the same brigade or in a follow up brigade) and we should continue doing so. Regards Rüdiger
Re: httpd 2.4.25, mpm_event, ssl: segfaults
On Mon, Feb 6, 2017 at 12:10 PM, Ruediger Pluem wrote: >> >> What might happen in ssl_io_filter_output() is that buffered >> output data (already deleted but not cleared) end up being reused >> on shutdown. >> >> Could you please try the attached patch? > > Why would we need to handle filter_ctx->pssl == NULL the same way we > handle META_BUCKETS? filter_ctx->pssl == NULL already causes > ssl_filter_write to fail. Do I miss any code before that could crash > in the data case with filter_ctx->pssl == NULL? No, this hunk was not intended to be proposed/tested (the case should not happen though, so harmless imo), and anyway was not committed in r1781582 ([1]). However I opened the thread "ssl_io_filter_output vs EOC" ([2]), maybe we could discuss this there? It seems that we can either error/fail or fall through the filter stack after the EOC, depending on whether subsequents buckets are in the same brigade or not. We probably should clarify (and being consistent on) what to do after the EOC when TLS is in place (i.e. send whatever follows, besides metadata, in clear or bail out?). Regards, Yann. [1] http://svn.apache.org/r1781582 [2] https://lists.apache.org/thread.html/714ca91c918e7520b75fae664b2bdee28d7b2a9f9ef78e51d8765c96@%3Cdev.httpd.apache.org%3E
Re: Configuration of trusted OCSP responder certificates
On Mon, Feb 6, 2017 at 12:09 PM, Yann Ylavic wrote: > On Mon, Feb 6, 2017 at 11:28 AM, Thijs Kinkhorst > wrote: >> On 02-01-17 14:17, Thijs Kinkhorst wrote: >>> I'd like to enquire about the possibilities to merge the patch to >>> support configuring trusted OCSP responder certificates. >>> >>> We need this change in order to be able to use OCSP with client >>> certificate authentication. >>> >>> The patch is in >>> https://bz.apache.org/bugzilla/show_bug.cgi?id=46037 >>> for a few years now and there have been several reports of it working >>> without problems; we're also running it for a few years and it works >>> fine for us. >> >> Is there anyone who can help me with this? Anything we can do? > > The patch was committed[1], and is being reviewed (possibly proposed > for backport to 2.4.x soon). > > > Regards, > Yann. > > [1] http://svn.apache.org/viewvc?r1781575 Sorry, bad link: http://svn.apache.org/r1781575
Re: httpd 2.4.25, mpm_event, ssl: segfaults
On 02/02/2017 11:04 AM, Yann Ylavic wrote: > Hi Niklas, > > On Wed, Feb 1, 2017 at 7:02 PM, Niklas Edmundsson wrote: >> >> We've started to see spurious segfaults with httpd 2.4.25, mpm_event, ssl on >> Ubuntu 14.04LTS. Not frequent, but none the less happening. >> >> #4 ssl_io_filter_output (f=0x7f507013cfe0, bb=0x7f4f840be168) at >> ssl_engine_io.c:1746 >> data = 0x7f5075518000 > 0x7f5075518000> >> len = 4194304 >> bucket = 0x7f4f840b1ba8 >> status = >> filter_ctx = 0x7f507013cf88 >> inctx = >> outctx = 0x7f507013d008 >> rblock = APR_NONBLOCK_READ > > I suspect some cleanup ordering issue happening in > ssl_io_filter_output(), when the EOC bucket is found. > >> >> Are we hitting a corner case of process cleanup that plays merry hell with >> https/ssl, or are we just having bad luck? Ideas? Suggestions? > > 2.4.25 is eager to terminate/shutdown keepalive connections more > quickly (than previous versions) on graceful shutdown (e.g. > MaxConnectionsPerChild reached). > > What might happen in ssl_io_filter_output() is that buffered output > data (already deleted but not cleared) end up being reused on > shutdown. > > Could you please try the attached patch? Why would we need to handle filter_ctx->pssl == NULL the same way we handle META_BUCKETS? filter_ctx->pssl == NULL already causes ssl_filter_write to fail. Do I miss any code before that could crash in the data case with filter_ctx->pssl == NULL? Regards Rüdiger
Re: Configuration of trusted OCSP responder certificates
On Mon, Feb 6, 2017 at 11:28 AM, Thijs Kinkhorst wrote: > On 02-01-17 14:17, Thijs Kinkhorst wrote: >> I'd like to enquire about the possibilities to merge the patch to >> support configuring trusted OCSP responder certificates. >> >> We need this change in order to be able to use OCSP with client >> certificate authentication. >> >> The patch is in >> https://bz.apache.org/bugzilla/show_bug.cgi?id=46037 >> for a few years now and there have been several reports of it working >> without problems; we're also running it for a few years and it works >> fine for us. > > Is there anyone who can help me with this? Anything we can do? The patch was committed[1], and is being reviewed (possibly proposed for backport to 2.4.x soon). Regards, Yann. [1] http://svn.apache.org/viewvc?r1781575
Re: mod_http2 and Frequent wake-ups for mpm_event
Hi Stefan,
On Mon, Feb 6, 2017 at 9:57 AM, Stefan Priebe - Profihost AG
wrote:
>
> your last patch results in multiple crashes every second:
Sorry about that, the changes in mpm_event were incorrect (the mutex
was cleared with the pool when recycled, hence its pointer was
dangling).
New patch attached, this time tested with the httpd framework (where
the previous patch segfaulted too).
Thanks,
Yann.
Index: server/mpm/event/event.c
===
--- server/mpm/event/event.c (revision 1781789)
+++ 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(&ptrans, worker_queue_info);
@@ -1762,8 +1764,10 @@ static void * APR_THREAD_FUNC listener_thread(apr_
signal_threads(ST_GRACEFUL);
return NULL;
}
+apr_pool_tag(ptrans, "transaction");
}
-apr_pool_tag(ptrans, "transaction");
+apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, ptrans);
+apr_allocator_mutex_set(apr_pool_allocator_get(ptrans), mutex);
get_worker(&have_idle_worker, 1, &workers_were_busy);
rc = lr->accept_func(&csd, lr, ptrans);
Index: modules/http2/h2_conn.c
===
--- modules/http2/h2_conn.c (revision 1781789)
+++ modules/http2/h2_conn.c (working copy)
@@ -247,9 +247,10 @@ apr_status_t h2_conn_pre_close(struct h2_ctx *ctx,
return status;
}
-conn_rec *h2_slave_create(conn_rec *master, int slave_id, apr_pool_t *parent)
+conn_rec *h2_slave_create(conn_rec *master, int slave_id)
{
apr_allocator_t *allocator;
+apr_thread_mutex_t *mutex;
apr_pool_t *pool;
conn_rec *c;
void *cfg;
@@ -264,9 +265,11 @@ apr_status_t h2_conn_pre_close(struct h2_ctx *ctx,
* another thread.
*/
apr_allocator_create(&allocator);
-apr_pool_create_ex(&pool, parent, NULL, allocator);
+apr_pool_create_ex(&pool, master->pool, NULL, allocator);
+apr_allocator_owner_set(allocator, pool);
apr_pool_tag(pool, "h2_slave_conn");
-apr_allocator_owner_set(allocator, pool);
+apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, pool);
+apr_allocator_mutex_set(allocator, mutex);
c = (conn_rec *) apr_palloc(pool, sizeof(conn_rec));
if (c == NULL) {
Index: modules/http2/h2_conn.h
===
--- modules/http2/h2_conn.h (revision 1781789)
+++ modules/http2/h2_conn.h (working copy)
@@ -66,7 +66,7 @@ typedef enum {
h2_mpm_type_t h2_conn_mpm_type(void);
-conn_rec *h2_slave_create(conn_rec *master, int slave_id, apr_pool_t *parent);
+conn_rec *h2_slave_create(conn_rec *master, int slave_id);
void h2_slave_destroy(conn_rec *slave);
apr_status_t h2_slave_run_pre_connection(conn_rec *slave, apr_socket_t *csd);
Index: modules/http2/h2_mplx.c
===
--- modules/http2/h2_mplx.c (revision 1781789)
+++ modules/http2/h2_mplx.c (working copy)
@@ -33,6 +33,7 @@
#include "h2_private.h"
#include "h2_bucket_beam.h"
#include "h2_config.h"
+#include "h2_session.h"
#include "h2_conn.h"
#include "h2_ctx.h"
#include "h2_h2.h"
@@ -245,7 +246,7 @@ static void h2_mplx_destroy(h2_mplx *m)
m->id, (int)h2_ihash_count(m->tasks));
check_tx_free(m);
/* pool will be destroyed as child of h2_session->pool,
- slave connection pools are children of m->pool */
+ slave connection pools are children of m->c->pool */
}
/**
@@ -259,32 +260,27 @@ static void h2_mplx_destroy(h2_mplx *m)
* their HTTP/1 cousins, the separate allocator seems to work better
* than protecting a shared h2_session one with an own lock.
*/
-h2_mplx *h2_mplx_create(conn_rec *c, apr_pool_t *parent,
-const h2_config *conf,
-apr_interval_time_t stream_timeout,
-h2_workers *workers)
+h2_mplx *h2_mplx_create(h2_session *session, h2_workers *workers)
{
apr_status_t status = APR_SUCCESS;
-apr_allocator_t *allocator = NULL;
+apr_pool_t *parent = session->pool;
+const h2_config *conf = session->config;
h2_mplx *m;
ap_assert(conf);
-status = apr_allocator_create(&allocator);
-if (status != APR_SUCCESS) {
-return NULL;
-}
-
m = apr_pcalloc(parent, sizeof(h2_mplx));
if (m) {
+conn_rec *c = session->c;
+
m->id = c->id;
APR_RING_ELEM_INIT(m, link);
m->c = c;
-apr
Re: Configuration of trusted OCSP responder certificates
On 02-01-17 14:17, Thijs Kinkhorst wrote: > I'd like to enquire about the possibilities to merge the patch to > support configuring trusted OCSP responder certificates. > > We need this change in order to be able to use OCSP with client > certificate authentication. > > The patch is in > https://bz.apache.org/bugzilla/show_bug.cgi?id=46037 > for a few years now and there have been several reports of it working > without problems; we're also running it for a few years and it works > fine for us. Is there anyone who can help me with this? Anything we can do? Thanks, Thijs signature.asc Description: OpenPGP digital signature
Re: mod_http2 and Frequent wake-ups for mpm_event
Thanks Yann (the one and only) for quickly adressing this. This explains maybe some of my frustrations in the past with making pools/allocators work in the h2 environment. -Stefan > Am 06.02.2017 um 01:19 schrieb Yann Ylavic : > > Hi Stefan, > > On Sun, Feb 5, 2017 at 7:51 PM, Stefan Priebe - Profihost AG > wrote: >> >> tested your patch against mod_ssl. I haven't seen any pool crashes again >> so it seems to fix this issue. >> >> But two new ones: > > Possibly a promising fix suggested (in another thread) by yet another > talented Stefan :) > He has spotted a possible issue in mpm_event which could affect mainly > mod_http2. > I applied his suggestion in the attached patch, and did some > extrapolation for similar code in mod_h2 (so all possible errors are > mine...). > Would you test this one please? > > @icing: I changed the parent pool of the slave connection from the > mplx pool to the master connection's (ptrans), just to simplify the > allocators in place for now. > I don't see it was needed from a concurrency POV, but if you wanted to > avoid locking when creating slaves' pool we can probably keep the > dedicated allocator finally (to reduce possible contention on > ptrans->mutex? No mutex needed I think since mplx doesn't seem to have > other subpools. Trade off with memory consumption, though). > > > Regards, > Yann. >
Re: mod_http2 and Frequent wake-ups for mpm_event
ok, I'll look at them. > Am 05.02.2017 um 19:51 schrieb Stefan Priebe - Profihost AG > : > > Hi Yann, > > tested your patch against mod_ssl. I haven't seen any pool crashes again > so it seems to fix this issue. > > But two new ones: > > Program terminated with signal SIGABRT, Aborted. > #0 0x7ff523558067 in raise () from /lib/x86_64-linux-gnu/libc.so.6 > #0 0x7ff523558067 in raise () from /lib/x86_64-linux-gnu/libc.so.6 > #1 0x7ff523559448 in abort () from /lib/x86_64-linux-gnu/libc.so.6 > #2 0x7ff5235961b4 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 > #3 0x7ff52359b98e in ?? () from /lib/x86_64-linux-gnu/libc.so.6 > #4 0x7ff52359c923 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 > #5 0x7ff523b06c83 in apr_allocator_destroy (allocator=0x7ff4de50) >at memory/unix/apr_pools.c:152 > #6 0x556e7986bbef in task_destroy (m=0x7ff46402a028, > task=0x7ff4d0029f00, >called_from_master=0) at h2_mplx.c:400 > #7 0x556e7986ce5b in task_done_iter (ctx=, >val=) at h2_mplx.c:1064 > #8 0x7ff523afe5e6 in apr_hash_do ( >comp=comp@entry=0x556e7987f180 , > rec=rec@entry=0x7ff5037e5480, >ht=) at tables/apr_hash.c:542 > #9 0x556e7987fb5f in h2_ihash_iter (ih=, >fn=fn@entry=0x556e7986ce50 , > ctx=ctx@entry=0x7ff46402a028) >at h2_util.c:315 > #10 0x556e7986d463 in h2_mplx_release_and_join (m=0x7ff46402a028, >wait=0x7ff464029fd0) at h2_mplx.c:619 > #11 0x556e79872ae4 in session_pool_cleanup (data=0x7ff464020318) >at h2_session.c:827 > #12 0x7ff523b0748e in run_cleanups (cref=0x7ff464029fa8) >at memory/unix/apr_pools.c:2352 > #13 apr_pool_destroy (pool=0x7ff464029f38) at memory/unix/apr_pools.c:804 > #14 0x7ff523b07745 in apr_pool_clear (pool=0x7ff4fc0150e8) >at memory/unix/apr_pools.c:769 > #15 0x556e798aa698 in ap_push_pool (queue_info=0x1e2e, >pool_to_recycle=0x1ebb) at fdqueue.c:234 > #16 0x556e798a59da in process_lingering_close (cs=0x7ff4fc015378, >pfd=0x556e7b8bd888) at event.c:1513 > #17 0x556e798a9510 in listener_thread (thd=0x1e2e, > dummy=0x547b44ea3e1b3) >at event.c:1837 > #18 0x7ff5238d60a4 in start_thread () > from /lib/x86_64-linux-gnu/libpthread.so.0 > #19 0x7ff52360b62d in clone () from /lib/x86_64-linux-gnu/libc.so.6 > > and > > Program terminated with signal SIGSEGV, Segmentation fault. > #0 apr_bucket_alloc (size=size@entry=64, > list=list@entry=0x3fb50d4a7aeb1d49) >at buckets/apr_buckets_alloc.c:128 > #0 apr_bucket_alloc (size=size@entry=64, > list=list@entry=0x3fb50d4a7aeb1d49) >at buckets/apr_buckets_alloc.c:128 > #1 0x7ff523d2b1d3 in apr_bucket_heap_create ( >buf=0x7ff51003b3a8 " > \311\021A\216y\034\347\034Wy\360\343R\275\226o\020iw\227r\337\377", > length=1300, free_func=0x7ff523d2ab50 , >list=0x3fb50d4a7aeb1d49) at buckets/apr_buckets_heap.c:81 > #2 0x556e79884f85 in append_scratch (io=0x7ff4440377c8) >at h2_conn_io.c:165 > #3 0x556e79884ffa in assure_scratch_space (io=0x7ff4440377c8) >at h2_conn_io.c:182 > #4 0x556e79885ce8 in h2_conn_io_pass (io=io@entry=0x7ff4440377c8, >bb=0x7ff444133698) at h2_conn_io.c:393 > #5 0x556e798732be in on_send_data_cb (ngh2=, >frame=, framehd=, length=1291, >source=, userp=0x7ff444037780) at h2_session.c:648 > #6 0x7ff5243dde95 in ?? () from > /usr/lib/x86_64-linux-gnu/libnghttp2.so.14 > #7 0x7ff5243deea9 in nghttp2_session_send () > from /usr/lib/x86_64-linux-gnu/libnghttp2.so.14 > #8 0x556e79875857 in h2_session_send (session=0x7ff444037780) >at h2_session.c:1376 > #9 0x556e79878b6c in h2_session_process (session=0x7ff444037780, >async=2062228809) at h2_session.c:2208 > #10 0x556e79867788 in h2_conn_run (ctx=0x7ff4440376b0, c=0x7ff51003b6a8) >at h2_conn.c:214 > #11 0x556e7986a421 in h2_h2_process_conn (c=0x7ff51003b6a8) at > h2_h2.c:658 > #12 0x556e7980d050 in ap_run_process_connection (c=0x7ff51003b6a8) >at connection.c:42 > #13 0x556e798a7590 in process_socket (my_thread_num=, >my_child_num=, cs=0x7ff51003b618, sock=, >p=, thd=) at event.c:1134 > #14 worker_thread (thd=0x40, dummy=0x3fb50d4a7aeb1d49) at event.c:2137 > #15 0x7ff5238d60a4 in start_thread () > from /lib/x86_64-linux-gnu/libpthread.so.0 > #16 0x7ff52360b62d in clone () from /lib/x86_64-linux-gnu/libc.so.6 > > Greets, > Stefan > >> Am 02.02.2017 um 11:09 schrieb Yann Ylavic: >> Hi Stefan, >> >> On Tue, Jan 31, 2017 at 4:01 PM, Stefan Priebe - Profihost AG >> wrote: >>> >>> any ideas? >> >> I wonder if the attached patch (related to mod_ssl and proposed for >> another segfault report) could help in your case. >> >> Would you mind give it a try? >> >> >> Thanks, >> Yann. >>
Re: mod_http2 and Frequent wake-ups for mpm_event
Hi Yann, your last patch results in multiple crashes every second: Program terminated with signal SIGSEGV, Segmentation fault. #0 apr_pool_cleanup_kill (p=0x7f2b, data=data@entry=0x7f2bb40478c0, cleanup_fn=cleanup_fn@entry=0x7f2bc92285b0 ) at memory/unix/apr_pools.c:2264 2264memory/unix/apr_pools.c: No such file or directory. #0 apr_pool_cleanup_kill (p=0x7f2b, data=data@entry=0x7f2bb40478c0, cleanup_fn=cleanup_fn@entry=0x7f2bc92285b0 ) at memory/unix/apr_pools.c:2264 #1 0x7f2bc9224871 in apr_pool_cleanup_run (p=, data=0x7f2bb40478c0, cleanup_fn=0x7f2bc92285b0 ) at memory/unix/apr_pools.c:2342 #2 0x7f2bc9228892 in apr_socket_close (thesocket=) at network_io/unix/sockets.c:183 #3 0x555d4f07b99a in process_lingering_close (cs=0x7f2bb4047ac8, pfd=0x555d4f75dfa8) at event.c:1510 #4 0x555d4f07f4e0 in listener_thread (thd=0x7f2b, dummy=0x547d8caf3534b) at event.c:1837 #5 0x7f2bc8ff20a4 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0 #6 0x7f2bc8d2762d in clone () from /lib/x86_64-linux-gnu/libc.so.6 Stefan Am 06.02.2017 um 09:33 schrieb Stefan Priebe - Profihost AG: > Hallo, > > up and running (apache 2.4.25 + mpm_event V7 + mpd_http2 v1.8.11 + > mod_ssl patch + your new allocator patch). Will report back. > > Greets, > Stefan > > Am 06.02.2017 um 01:19 schrieb Yann Ylavic: >> Hi Stefan, >> >> On Sun, Feb 5, 2017 at 7:51 PM, Stefan Priebe - Profihost AG >> wrote: >>> >>> tested your patch against mod_ssl. I haven't seen any pool crashes again >>> so it seems to fix this issue. >>> >>> But two new ones: >> >> Possibly a promising fix suggested (in another thread) by yet another >> talented Stefan :) >> He has spotted a possible issue in mpm_event which could affect mainly >> mod_http2. >> I applied his suggestion in the attached patch, and did some >> extrapolation for similar code in mod_h2 (so all possible errors are >> mine...). >> Would you test this one please? >> >> @icing: I changed the parent pool of the slave connection from the >> mplx pool to the master connection's (ptrans), just to simplify the >> allocators in place for now. >> I don't see it was needed from a concurrency POV, but if you wanted to >> avoid locking when creating slaves' pool we can probably keep the >> dedicated allocator finally (to reduce possible contention on >> ptrans->mutex? No mutex needed I think since mplx doesn't seem to have >> other subpools. Trade off with memory consumption, though). >> >> >> Regards, >> Yann. >>
Re: mod_http2 and Frequent wake-ups for mpm_event
Hallo, up and running (apache 2.4.25 + mpm_event V7 + mpd_http2 v1.8.11 + mod_ssl patch + your new allocator patch). Will report back. Greets, Stefan Am 06.02.2017 um 01:19 schrieb Yann Ylavic: > Hi Stefan, > > On Sun, Feb 5, 2017 at 7:51 PM, Stefan Priebe - Profihost AG > wrote: >> >> tested your patch against mod_ssl. I haven't seen any pool crashes again >> so it seems to fix this issue. >> >> But two new ones: > > Possibly a promising fix suggested (in another thread) by yet another > talented Stefan :) > He has spotted a possible issue in mpm_event which could affect mainly > mod_http2. > I applied his suggestion in the attached patch, and did some > extrapolation for similar code in mod_h2 (so all possible errors are > mine...). > Would you test this one please? > > @icing: I changed the parent pool of the slave connection from the > mplx pool to the master connection's (ptrans), just to simplify the > allocators in place for now. > I don't see it was needed from a concurrency POV, but if you wanted to > avoid locking when creating slaves' pool we can probably keep the > dedicated allocator finally (to reduce possible contention on > ptrans->mutex? No mutex needed I think since mplx doesn't seem to have > other subpools. Trade off with memory consumption, though). > > > Regards, > Yann. >
