Re: pool debugging and httpd HTTP/2
On 17.01.2019 15:05, Yann Ylavic wrote: > On Thu, Jan 17, 2019 at 2:55 PM Stefan Eissing > wrote: >> >>> Am 17.01.2019 um 14:04 schrieb Yann Ylavic : >>> >>> On Thu, Jan 17, 2019 at 1:56 PM Branko Čibej wrote: On 17.01.2019 13:55, Branko Čibej wrote: > On 17.01.2019 13:21, Yann Ylavic wrote: >> On Thu, Jan 17, 2019 at 1:02 PM Yann Ylavic wrote: >>> On Thu, Jan 17, 2019 at 12:50 PM Branko Čibej wrote: Other than that, unlimited max-free is wrong in most cases, so why not set the default to something sane instead? >>> Agreed, something like 10 pools (80K)? >> Probably at bit too agressive... >> >> In httpd that's 2MB, which default is used in svn? > ./subversion/include/svn_pools.h Huh, something broke my MUA ... there should be some more on that line: #define SVN_ALLOCATOR_RECOMMENDED_MAX_FREE (4096 * 1024) >>> OK, so I'd propose 3072 * 1024 :) >> And so, the binary search for the optimal value started... > Dichotomy is not the worst search algorithm, though :) > > I suppose there is as much optimal value as applications though, on > the APR side it's more of a safeguard than anything el Indeed. Our documentation should recommend to downstream users to set the max-free to a value that's reasonable for their application. It wouldn't hurt to hint at what the tradeoffs are. -- Brane > se I suppose.
Re: pool debugging and httpd HTTP/2
On Thu, Jan 17, 2019 at 2:55 PM Stefan Eissing wrote: > > > > Am 17.01.2019 um 14:04 schrieb Yann Ylavic : > > > > On Thu, Jan 17, 2019 at 1:56 PM Branko Čibej wrote: > >> > >> On 17.01.2019 13:55, Branko Čibej wrote: > >>> On 17.01.2019 13:21, Yann Ylavic wrote: > On Thu, Jan 17, 2019 at 1:02 PM Yann Ylavic wrote: > > On Thu, Jan 17, 2019 at 12:50 PM Branko Čibej wrote: > >> Other than that, unlimited max-free is wrong in most cases, so why not > >> set the default to something sane instead? > > Agreed, something like 10 pools (80K)? > Probably at bit too agressive... > > In httpd that's 2MB, which default is used in svn? > >>> ./subversion/include/svn_pools.h > >> > >> Huh, something broke my MUA ... there should be some more on that line: > >> > >> #define SVN_ALLOCATOR_RECOMMENDED_MAX_FREE (4096 * 1024) > > > > OK, so I'd propose 3072 * 1024 :) > > And so, the binary search for the optimal value started... Dichotomy is not the worst search algorithm, though :) I suppose there is as much optimal value as applications though, on the APR side it's more of a safeguard than anything else I suppose.
Re: pool debugging and httpd HTTP/2
> Am 17.01.2019 um 14:04 schrieb Yann Ylavic : > > On Thu, Jan 17, 2019 at 1:56 PM Branko Čibej wrote: >> >> On 17.01.2019 13:55, Branko Čibej wrote: >>> On 17.01.2019 13:21, Yann Ylavic wrote: On Thu, Jan 17, 2019 at 1:02 PM Yann Ylavic wrote: > On Thu, Jan 17, 2019 at 12:50 PM Branko Čibej wrote: >> Other than that, unlimited max-free is wrong in most cases, so why not >> set the default to something sane instead? > Agreed, something like 10 pools (80K)? Probably at bit too agressive... In httpd that's 2MB, which default is used in svn? >>> ./subversion/include/svn_pools.h >> >> Huh, something broke my MUA ... there should be some more on that line: >> >> #define SVN_ALLOCATOR_RECOMMENDED_MAX_FREE (4096 * 1024) > > OK, so I'd propose 3072 * 1024 :) And so, the binary search for the optimal value started...
Re: pool debugging and httpd HTTP/2
On Thu, Jan 17, 2019 at 1:56 PM Branko Čibej wrote: > > On 17.01.2019 13:55, Branko Čibej wrote: > > On 17.01.2019 13:21, Yann Ylavic wrote: > >> On Thu, Jan 17, 2019 at 1:02 PM Yann Ylavic wrote: > >>> On Thu, Jan 17, 2019 at 12:50 PM Branko Čibej wrote: > Other than that, unlimited max-free is wrong in most cases, so why not > set the default to something sane instead? > >>> Agreed, something like 10 pools (80K)? > >> Probably at bit too agressive... > >> > >> In httpd that's 2MB, which default is used in svn? > > ./subversion/include/svn_pools.h > > Huh, something broke my MUA ... there should be some more on that line: > > #define SVN_ALLOCATOR_RECOMMENDED_MAX_FREE (4096 * 1024) OK, so I'd propose 3072 * 1024 :)
Re: pool debugging and httpd HTTP/2
On 17.01.2019 13:11, Yann Ylavic wrote: > On Thu, Jan 17, 2019 at 1:09 PM Stefan Sperling wrote: >> On Thu, Jan 17, 2019 at 12:36:49PM +0100, Yann Ylavic wrote: >>> OK, so an APR only option like this? >> Yes, thank you, this looks great! >> I'll see about making OpenBSD's APR port use this patch. > As discussed with Brane, you may want to noop > apr_allocator_max_free_set() too, otherwise each APR-ed application is > free to use it's own limit at runtime... Or at least change it so that applications can't set max-free to unlimited. -- Bane
Re: pool debugging and httpd HTTP/2
On 17.01.2019 13:55, Branko Čibej wrote: > On 17.01.2019 13:21, Yann Ylavic wrote: >> On Thu, Jan 17, 2019 at 1:02 PM Yann Ylavic wrote: >>> On Thu, Jan 17, 2019 at 12:50 PM Branko Čibej wrote: Other than that, unlimited max-free is wrong in most cases, so why not set the default to something sane instead? >>> Agreed, something like 10 pools (80K)? >> Probably at bit too agressive... >> >> In httpd that's 2MB, which default is used in svn? > ./subversion/include/svn_pools.h Huh, something broke my MUA ... there should be some more on that line: #define SVN_ALLOCATOR_RECOMMENDED_MAX_FREE (4096 * 1024) -- Brane
Re: pool debugging and httpd HTTP/2
On 17.01.2019 13:21, Yann Ylavic wrote: > On Thu, Jan 17, 2019 at 1:02 PM Yann Ylavic wrote: >> On Thu, Jan 17, 2019 at 12:50 PM Branko Čibej wrote: >>> Other than that, unlimited max-free is wrong in most cases, so why not >>> set the default to something sane instead? >> Agreed, something like 10 pools (80K)? > Probably at bit too agressive... > > In httpd that's 2MB, which default is used in svn? ./subversion/include/svn_pools.h
Re: pool debugging and httpd HTTP/2
On Thu, Jan 17, 2019 at 1:02 PM Yann Ylavic wrote: > > On Thu, Jan 17, 2019 at 12:50 PM Branko Čibej wrote: > > > > Other than that, unlimited max-free is wrong in most cases, so why not > > set the default to something sane instead? > > Agreed, something like 10 pools (80K)? Probably at bit too agressive... In httpd that's 2MB, which default is used in svn?
Re: pool debugging and httpd HTTP/2
On Thu, Jan 17, 2019 at 01:02:15PM +0100, Yann Ylavic wrote: > On Thu, Jan 17, 2019 at 12:50 PM Branko Čibej wrote: > > > > On 17.01.2019 12:36, Yann Ylavic wrote: > > > OK, so an APR only option like this? > > > > This still affects /everything/ that uses this particular compiled > > version of APR, doesn't it? > > Sure, but AIUI this is Stefan's goal.. Yes indeed. The fact that APR applications can still override the setting is a problem. I will enhance the proposed patch to disallow changing it. > > Except Subversion, which sets its own > > max_free on allocators it creates. > > Yes, since it's settable at runtime too, I suppose OpenBSD would have > to noop apr_allocator_max_free_set() for global applicability... Their > choice. Yes, thanks for the hint.
Re: pool debugging and httpd HTTP/2
On Thu, Jan 17, 2019 at 1:09 PM Stefan Sperling wrote: > > On Thu, Jan 17, 2019 at 12:36:49PM +0100, Yann Ylavic wrote: > > OK, so an APR only option like this? > > Yes, thank you, this looks great! > I'll see about making OpenBSD's APR port use this patch. As discussed with Brane, you may want to noop apr_allocator_max_free_set() too, otherwise each APR-ed application is free to use it's own limit at runtime...
Re: pool debugging and httpd HTTP/2
On Thu, Jan 17, 2019 at 12:36:49PM +0100, Yann Ylavic wrote: > OK, so an APR only option like this? Yes, thank you, this looks great! I'll see about making OpenBSD's APR port use this patch.
Re: pool debugging and httpd HTTP/2
On Thu, Jan 17, 2019 at 12:50 PM Branko Čibej wrote: > > On 17.01.2019 12:36, Yann Ylavic wrote: > > OK, so an APR only option like this? > > This still affects /everything/ that uses this particular compiled > version of APR, doesn't it? Sure, but AIUI this is Stefan's goal.. > Except Subversion, which sets its own > max_free on allocators it creates. Yes, since it's settable at runtime too, I suppose OpenBSD would have to noop apr_allocator_max_free_set() for global applicability... Their choice. > > Other than that, unlimited max-free is wrong in most cases, so why not > set the default to something sane instead? Agreed, something like 10 pools (80K)? Regards, Yann.
Re: pool debugging and httpd HTTP/2
On 17.01.2019 12:36, Yann Ylavic wrote:
> On Thu, Jan 17, 2019 at 12:10 PM Stefan Sperling wrote:
>> On Thu, Jan 17, 2019 at 12:04:37PM +0100, Yann Ylavic wrote:
>>> On Tue, Jan 15, 2019 at 11:48 AM Stefan Sperling wrote:
On Tue, Jan 15, 2019 at 11:19:24AM +0100, Stefan Eissing wrote:
> Would OpenBSD be happy with a setting (COMPILE FLAG) that forces
> the immediate free() by allocators and otherwise skipping the DEBUG
> flags?
Yes, I think that would be great. I don't care about the pool debugging
aspects as much as the benefits we get from direct use of our nalloc/free.
>>> How about "MaxMemFree 1" in OpenBSD's httpd then instead of compile
>>> time APR_POOL_DEBUG?
>> Thanks for the suggestion!
>>
>> Unfortunately, this would not help other applications using APR,
>> such as Subversion clients. I'd rather not deploy application-level
>> workarounds for behaviour of a library used by these applications.
> OK, so an APR only option like this?
>
> Index: srclib/apr/include/apr_allocator.h
> ===
> --- srclib/apr/include/apr_allocator.h(revision 1800753)
> +++ srclib/apr/include/apr_allocator.h(working copy)
> @@ -65,6 +65,9 @@ struct apr_memnode_t {
>
> /** Symbolic constants */
> #define APR_ALLOCATOR_MAX_FREE_UNLIMITED 0
> +#ifndef APR_ALLOCATOR_MAX_FREE_DEFAULT
> +#define APR_ALLOCATOR_MAX_FREE_DEFAULT APR_ALLOCATOR_MAX_FREE_UNLIMITED
> +#endif
>
> /**
> * Create a new allocator
> Index: srclib/apr/memory/unix/apr_pools.c
> ===
> --- srclib/apr/memory/unix/apr_pools.c(revision 1800753)
> +++ srclib/apr/memory/unix/apr_pools.c(working copy)
> @@ -127,8 +127,8 @@ struct apr_allocator_t {
> apr_size_tmax_index;
> /** Total size (in BOUNDARY_SIZE multiples) of unused memory before
> * blocks are given back. @see apr_allocator_max_free_set().
> - * @note Initialized to APR_ALLOCATOR_MAX_FREE_UNLIMITED,
> - * which means to never give back blocks.
> + * @note Initialized to APR_ALLOCATOR_MAX_FREE_DEFAULT,
> + * which by default means to never give back blocks.
> */
> apr_size_tmax_free_index;
> /**
> @@ -170,7 +170,7 @@ APR_DECLARE(apr_status_t) apr_allocator_create(apr
> return APR_ENOMEM;
>
> memset(new_allocator, 0, SIZEOF_ALLOCATOR_T);
> -new_allocator->max_free_index = APR_ALLOCATOR_MAX_FREE_UNLIMITED;
> +new_allocator->max_free_index = APR_ALLOCATOR_MAX_FREE_DEFAULT;
>
> *allocator = new_allocator;
>
> @@ -1180,7 +1180,7 @@ APR_DECLARE(apr_status_t) apr_pool_create_unmanage
> return APR_ENOMEM;
> }
> memset(pool_allocator, 0, SIZEOF_ALLOCATOR_T);
> -pool_allocator->max_free_index = APR_ALLOCATOR_MAX_FREE_UNLIMITED;
> +pool_allocator->max_free_index = APR_ALLOCATOR_MAX_FREE_DEFAULT;
> }
> if ((node = allocator_alloc(pool_allocator,
> MIN_ALLOC - APR_MEMNODE_T_SIZE)) == NULL) {
> --
>
> For "-D APR_ALLOCATOR_MAX_FREE_DEFAULT=.." to be usable at build time.
> That one can possibly be upstreamed too.
This still affects /everything/ that uses this particular compiled
version of APR, doesn't it? Except Subversion, which sets its own
max_free on allocators it creates.
Other than that, unlimited max-free is wrong in most cases, so why not
set the default to something sane instead?
-- Brane
Re: pool debugging and httpd HTTP/2
On Thu, Jan 17, 2019 at 12:10 PM Stefan Sperling wrote:
>
> On Thu, Jan 17, 2019 at 12:04:37PM +0100, Yann Ylavic wrote:
> > On Tue, Jan 15, 2019 at 11:48 AM Stefan Sperling wrote:
> > >
> > > On Tue, Jan 15, 2019 at 11:19:24AM +0100, Stefan Eissing wrote:
> > > > Would OpenBSD be happy with a setting (COMPILE FLAG) that forces
> > > > the immediate free() by allocators and otherwise skipping the DEBUG
> > > > flags?
> > >
> > > Yes, I think that would be great. I don't care about the pool debugging
> > > aspects as much as the benefits we get from direct use of our nalloc/free.
> >
> > How about "MaxMemFree 1" in OpenBSD's httpd then instead of compile
> > time APR_POOL_DEBUG?
>
> Thanks for the suggestion!
>
> Unfortunately, this would not help other applications using APR,
> such as Subversion clients. I'd rather not deploy application-level
> workarounds for behaviour of a library used by these applications.
OK, so an APR only option like this?
Index: srclib/apr/include/apr_allocator.h
===
--- srclib/apr/include/apr_allocator.h(revision 1800753)
+++ srclib/apr/include/apr_allocator.h(working copy)
@@ -65,6 +65,9 @@ struct apr_memnode_t {
/** Symbolic constants */
#define APR_ALLOCATOR_MAX_FREE_UNLIMITED 0
+#ifndef APR_ALLOCATOR_MAX_FREE_DEFAULT
+#define APR_ALLOCATOR_MAX_FREE_DEFAULT APR_ALLOCATOR_MAX_FREE_UNLIMITED
+#endif
/**
* Create a new allocator
Index: srclib/apr/memory/unix/apr_pools.c
===
--- srclib/apr/memory/unix/apr_pools.c(revision 1800753)
+++ srclib/apr/memory/unix/apr_pools.c(working copy)
@@ -127,8 +127,8 @@ struct apr_allocator_t {
apr_size_tmax_index;
/** Total size (in BOUNDARY_SIZE multiples) of unused memory before
* blocks are given back. @see apr_allocator_max_free_set().
- * @note Initialized to APR_ALLOCATOR_MAX_FREE_UNLIMITED,
- * which means to never give back blocks.
+ * @note Initialized to APR_ALLOCATOR_MAX_FREE_DEFAULT,
+ * which by default means to never give back blocks.
*/
apr_size_tmax_free_index;
/**
@@ -170,7 +170,7 @@ APR_DECLARE(apr_status_t) apr_allocator_create(apr
return APR_ENOMEM;
memset(new_allocator, 0, SIZEOF_ALLOCATOR_T);
-new_allocator->max_free_index = APR_ALLOCATOR_MAX_FREE_UNLIMITED;
+new_allocator->max_free_index = APR_ALLOCATOR_MAX_FREE_DEFAULT;
*allocator = new_allocator;
@@ -1180,7 +1180,7 @@ APR_DECLARE(apr_status_t) apr_pool_create_unmanage
return APR_ENOMEM;
}
memset(pool_allocator, 0, SIZEOF_ALLOCATOR_T);
-pool_allocator->max_free_index = APR_ALLOCATOR_MAX_FREE_UNLIMITED;
+pool_allocator->max_free_index = APR_ALLOCATOR_MAX_FREE_DEFAULT;
}
if ((node = allocator_alloc(pool_allocator,
MIN_ALLOC - APR_MEMNODE_T_SIZE)) == NULL) {
--
For "-D APR_ALLOCATOR_MAX_FREE_DEFAULT=.." to be usable at build time.
That one can possibly be upstreamed too.
Re: pool debugging and httpd HTTP/2
On Thu, Jan 17, 2019 at 12:04:37PM +0100, Yann Ylavic wrote: > On Tue, Jan 15, 2019 at 11:48 AM Stefan Sperling wrote: > > > > On Tue, Jan 15, 2019 at 11:19:24AM +0100, Stefan Eissing wrote: > > > Would OpenBSD be happy with a setting (COMPILE FLAG) that forces > > > the immediate free() by allocators and otherwise skipping the DEBUG > > > flags? > > > > Yes, I think that would be great. I don't care about the pool debugging > > aspects as much as the benefits we get from direct use of our nalloc/free. > > How about "MaxMemFree 1" in OpenBSD's httpd then instead of compile > time APR_POOL_DEBUG? Thanks for the suggestion! Unfortunately, this would not help other applications using APR, such as Subversion clients. I'd rather not deploy application-level workarounds for behaviour of a library used by these applications.
Re: pool debugging and httpd HTTP/2
On Tue, Jan 15, 2019 at 11:48 AM Stefan Sperling wrote:
>
> On Tue, Jan 15, 2019 at 11:19:24AM +0100, Stefan Eissing wrote:
> > Would OpenBSD be happy with a setting (COMPILE FLAG) that forces
> > the immediate free() by allocators and otherwise skipping the DEBUG
> > flags?
>
> Yes, I think that would be great. I don't care about the pool debugging
> aspects as much as the benefits we get from direct use of our nalloc/free.
How about "MaxMemFree 1" in OpenBSD's httpd then instead of compile
time APR_POOL_DEBUG?
A patch could help enforcing this, something like:
Index: server/mpm_common.c
===
--- server/mpm_common.c(revision 1851349)
+++ server/mpm_common.c(working copy)
@@ -151,7 +151,9 @@ AP_DECLARE_DATA int ap_graceful_shutdown_timeout;
AP_DECLARE_DATA apr_uint32_t ap_max_mem_free;
AP_DECLARE_DATA apr_size_t ap_thread_stacksize;
+#ifndef ALLOCATOR_MAX_FREE_DEFAULT
#define ALLOCATOR_MAX_FREE_DEFAULT (2048*1024)
+#endif
/* Set defaults for config directives implemented here. This is
* called from core's pre-config hook, so MPMs which need to override
@@ -377,6 +379,7 @@ AP_DECLARE(const char *)ap_mpm_set_graceful_shutdo
const char *ap_mpm_set_max_mem_free(cmd_parms *cmd, void *dummy,
const char *arg)
{
+#ifndef __OpenBSD__
long value;
const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
if (err != NULL) {
@@ -392,6 +395,9 @@ const char *ap_mpm_set_max_mem_free(cmd_parms *cmd
ap_max_mem_free = (apr_uint32_t)value * 1024;
return NULL;
+#else
+return "MaxMemFree can't be used on OpenBSD";
+#endif
}
The first hunk is probably something that can be upstreamed (for "-D
ALLOCATOR_MAX_FREE_DEFAULT=1" to be usable at build time), the second
hunk is up to you...
>
> > Personally, I would like that to be the default behaviour in httpd. The
> > current implementation seems to hinder use of modern address sanitation
> > tools and fuzzing. Something which has proven very valuable in h2.
I disagree for the default, it's a trade off as always.
>
> Agreed. Note that OpenBSD's malloc/free provide a degree of instrumentation
> similar to address sanitizer. This behaviour is tunable, and the default mode
> is not the most aggressive one. But when applications don't call free when
> they no longer use a chunk of memory, such instrumentation built into the
> system cannot work. See https://man.openbsd.org/malloc#MALLOC_OPTIONS
OpenBSD trades performance for its level of security, but it's not the
main concern for every one.
I'm still waiting for an OS allocator as performant as APR (or
another) pool allocator, different constraints between the two of
course. The OSes themselves use pool/bulk allocation internally when
performance matters...
PS: Stefan/icing, I'm happy to help with POOL_DEBUG , not much spare
time lately (new day job$) but I can try...
Re: pool debugging and httpd HTTP/2
On Tue, Jan 15, 2019 at 12:04:03PM +0100, Branko Čibej wrote: > While I understand all these arguments, I have trouble understanding how > they pertain to APR pools -- since there's no apr_pool_free(), the only > time memory can be returned to the system is during apr_pool_clear() and > apr_pool_destroy(). AFAIR in "production" mode, only the latter will > release memory, whereas apr_pool_clear() will just recycle it. Does > apr_pool_clear() release memory when pool debugging is enabled? As far as I can see, yes, pool_clear_debug() calls free() directly instead of allocator_free().
Re: pool debugging and httpd HTTP/2
On 15.01.2019 11:48, Stefan Sperling wrote: > On Tue, Jan 15, 2019 at 11:19:24AM +0100, Stefan Eissing wrote: >> Stefan: which DEBUG flags are "you" using in production for OpenBSD? I >> would like to run some h2 tests in exactly that setting... > We pass --enable-pool-debug=yes to the configure script. That's all. > >> Would OpenBSD be happy with a setting (COMPILE FLAG) that forces >> the immediate free() by allocators and otherwise skipping the DEBUG >> flags? > Yes, I think that would be great. I don't care about the pool debugging > aspects as much as the benefits we get from direct use of our nalloc/free. > >> Personally, I would like that to be the default behaviour in httpd. The >> current implementation seems to hinder use of modern address sanitation >> tools and fuzzing. Something which has proven very valuable in h2. > Agreed. Note that OpenBSD's malloc/free provide a degree of instrumentation > similar to address sanitizer. This behaviour is tunable, and the default mode > is not the most aggressive one. But when applications don't call free when > they no longer use a chunk of memory, such instrumentation built into the > system cannot work. See https://man.openbsd.org/malloc#MALLOC_OPTIONS While I understand all these arguments, I have trouble understanding how they pertain to APR pools -- since there's no apr_pool_free(), the only time memory can be returned to the system is during apr_pool_clear() and apr_pool_destroy(). AFAIR in "production" mode, only the latter will release memory, whereas apr_pool_clear() will just recycle it. Does apr_pool_clear() release memory when pool debugging is enabled? -- Brane
Re: pool debugging and httpd HTTP/2
On Tue, Jan 15, 2019 at 11:19:24AM +0100, Stefan Eissing wrote: > Stefan: which DEBUG flags are "you" using in production for OpenBSD? I > would like to run some h2 tests in exactly that setting... We pass --enable-pool-debug=yes to the configure script. That's all. > Would OpenBSD be happy with a setting (COMPILE FLAG) that forces > the immediate free() by allocators and otherwise skipping the DEBUG > flags? Yes, I think that would be great. I don't care about the pool debugging aspects as much as the benefits we get from direct use of our nalloc/free. > Personally, I would like that to be the default behaviour in httpd. The > current implementation seems to hinder use of modern address sanitation > tools and fuzzing. Something which has proven very valuable in h2. Agreed. Note that OpenBSD's malloc/free provide a degree of instrumentation similar to address sanitizer. This behaviour is tunable, and the default mode is not the most aggressive one. But when applications don't call free when they no longer use a chunk of memory, such instrumentation built into the system cannot work. See https://man.openbsd.org/malloc#MALLOC_OPTIONS
Re: pool debugging and httpd HTTP/2
First of all, thanks for helping me in this! > Am 14.01.2019 um 21:32 schrieb William A Rowe Jr : > > On Mon, Jan 14, 2019 at 12:56 PM Stefan Sperling wrote: > On Mon, Jan 14, 2019 at 11:38:55AM -0600, William A Rowe Jr wrote: > > On Mon, Jan 14, 2019 at 8:42 AM Stefan Sperling wrote: > > > > > > FYI, the reason APR pool debugging is enabled in production on OpenBSD > > > is because, after Heartbleed, OpenBSD decided to force 3rd party software > > > to use the operating system's malloc/free implementation instead of custom > > > allocators, where possible. > > > > > > If there was another way to make APR pools map directly to malloc/free > > > I would like to know about it. As far as I undestand, APR pools will > > > cache freed memory unless pool debugging is enabled. > > > > Your assessment is correct and by design. The argument by OpenBSD > > makes as much sense as attempting to force C construction/destruction > > on C++ sources, and OpenBSD users should expect side-effects of this > > rather radical change. APR pool Debugging has never been tested for > > the same level of robustness as the 'release' pool mechanics, nor have > > the many applications which are built upon APR pools. OpenBSD needs > > to consider their "port" of these many APR-consuming applications as > > independently maintained. > > >Well, I am quite happy with it as it is forcing me to fix bugs such as this > >one which nobody cared to fix for a decade (the quoted comment was written > >by gstein before SVN 1.0): https://svn.apache.org/r1850651 > > Yes, there are absolutely benefits to testing code under the AP_POOL_DEBUG > logic. My comments speak to the amount of testing and scrutiny that the > pool debug logic itself has been subjected to. I understand the benefits, but the production use really bites me. It means that I need to double my test builds and runs. That cuts into time I'd rather invest in other things. Stefan: which DEBUG flags are "you" using in production for OpenBSD? I would like to run some h2 tests in exactly that setting... > It seems that the mode was overloaded with too many meanings. If I had > to break them all out, they might look like; > > Typical allocator / subpool default behavior > Implicit allocator per subpool > > Typical allocator default reuse behavior > Free on pool release > Lock no-read/no-write on pool release (debugging: no address reuse) > > And re-implement the previously broken apr_pool_lock() for read-only > access to write-once/nonvolatile-on-fork pools such as config data. > > Then expand the API to permit some sort of apr_pool_split (_fork?) > as a complement to apr_pool_join. Expand the API to indicate the > handoff of the allocator from one thread to another or indicate that > the pool is free-threaded. Since APR will need to remain "stable" for existing applications, we seem to talk bout API extensions that allow apps like mod_http2 to specify their pool flavour more clearly. And that will then influence the debug code assertions and tests. But some current debug behaviour might need fixing before that, as I see the case with the current thread assertions. That seems to be over-optimized for a specific (but common) use case. > The apr_allocator_max_free_set() value of 0 was wrong. Zero should > always force no-reuse. -1 or similar should have been used for some > unlimited allocation reuse. A value of 1 today effectively does the same > thing as the correct implementation of 0. Would OpenBSD be happy with a setting (COMPILE FLAG) that forces the immediate free() by allocators and otherwise skipping the DEBUG flags? Personally, I would like that to be the default behaviour in httpd. The current implementation seems to hinder use of modern address sanitation tools and fuzzing. Something which has proven very valuable in h2. > Just some thoughts to start off discussion. Cheers, Stefan
Re: pool debugging and httpd HTTP/2
On Mon, Jan 14, 2019 at 12:56 PM Stefan Sperling wrote: > On Mon, Jan 14, 2019 at 11:38:55AM -0600, William A Rowe Jr wrote: > > On Mon, Jan 14, 2019 at 8:42 AM Stefan Sperling wrote: > > > > > > FYI, the reason APR pool debugging is enabled in production on OpenBSD > > > is because, after Heartbleed, OpenBSD decided to force 3rd party > software > > > to use the operating system's malloc/free implementation instead of > custom > > > allocators, where possible. > > > > > > If there was another way to make APR pools map directly to malloc/free > > > I would like to know about it. As far as I undestand, APR pools will > > > cache freed memory unless pool debugging is enabled. > > > > Your assessment is correct and by design. The argument by OpenBSD > > makes as much sense as attempting to force C construction/destruction > > on C++ sources, and OpenBSD users should expect side-effects of this > > rather radical change. APR pool Debugging has never been tested for > > the same level of robustness as the 'release' pool mechanics, nor have > > the many applications which are built upon APR pools. OpenBSD needs > > to consider their "port" of these many APR-consuming applications as > > independently maintained. > > Well, I am quite happy with it as it is forcing me to fix bugs such as this > one which nobody cared to fix for a decade (the quoted comment was written > by gstein before SVN 1.0): https://svn.apache.org/r1850651 Yes, there are absolutely benefits to testing code under the AP_POOL_DEBUG logic. My comments speak to the amount of testing and scrutiny that the pool debug logic itself has been subjected to. It seems that the mode was overloaded with too many meanings. If I had to break them all out, they might look like; Typical allocator / subpool default behavior Implicit allocator per subpool Typical allocator default reuse behavior Free on pool release Lock no-read/no-write on pool release (debugging: no address reuse) And re-implement the previously broken apr_pool_lock() for read-only access to write-once/nonvolatile-on-fork pools such as config data. Then expand the API to permit some sort of apr_pool_split (_fork?) as a complement to apr_pool_join. Expand the API to indicate the handoff of the allocator from one thread to another or indicate that the pool is free-threaded. The apr_allocator_max_free_set() value of 0 was wrong. Zero should always force no-reuse. -1 or similar should have been used for some unlimited allocation reuse. A value of 1 today effectively does the same thing as the correct implementation of 0. Just some thoughts to start off discussion.
Re: pool debugging and httpd HTTP/2
On Mon, Jan 14, 2019 at 11:38:55AM -0600, William A Rowe Jr wrote: > On Mon, Jan 14, 2019 at 8:42 AM Stefan Sperling wrote: > > > > > FYI, the reason APR pool debugging is enabled in production on OpenBSD > > is because, after Heartbleed, OpenBSD decided to force 3rd party software > > to use the operating system's malloc/free implementation instead of custom > > allocators, where possible. > > > > If there was another way to make APR pools map directly to malloc/free > > I would like to know about it. As far as I undestand, APR pools will > > cache freed memory unless pool debugging is enabled. > > > > Your assessment is correct and by design. The argument by OpenBSD > makes as much sense as attempting to force C construction/destruction > on C++ sources, and OpenBSD users should expect side-effects of this > rather radical change. APR pool Debugging has never been tested for > the same level of robustness as the 'release' pool mechanics, nor have > the many applications which are built upon APR pools. OpenBSD needs > to consider their "port" of these many APR-consuming applications as > independently maintained. Well, I am quite happy with it as it is forcing me to fix bugs such as this one which nobody cared to fix for a decade (the quoted comment was written by gstein before SVN 1.0): https://svn.apache.org/r1850651
Re: pool debugging and httpd HTTP/2
On Mon, Jan 14, 2019 at 03:26:41PM +0100, Stefan Eissing wrote: > Dear APR devs, > > I need help regarding apr pools and the assumptions they make, especially in > debug mode. > > Background: there are reports of read after free and failed assertions when > httpd's HTTP/2 implementation is used with pool debugging. > > From scanning the code, I can see two issues at least: > 1. pool debugging will on certain events traverse the pool tree. This seems > not a good idea as h2 may have child pools active in several threads. And > there is no global mutex for all pools - that is unwanted. > 2. The thread creating the pool is checked against the current thread in > apr_pool_check_integrity() which is certainly not true all the time in h2. > Pools are created in one thread and then passed to another. > > Now, I am not asking for revolutionary changes in apr pools to cope with my > implementation. But to change my implementation to work in pool debugging, I > need to understand the restrictions and underlying assumptions clearly. > > Before dumping the h2 pool usage description on the list here, maybe there is > someone willing to work with me on this closely? If the list prefers a > detailed discussion here, I can of course also do that. > > Please advise. > > Cheers, Stefan FYI, the reason APR pool debugging is enabled in production on OpenBSD is because, after Heartbleed, OpenBSD decided to force 3rd party software to use the operating system's malloc/free implementation instead of custom allocators, where possible. If there was another way to make APR pools map directly to malloc/free I would like to know about it. As far as I undestand, APR pools will cache freed memory unless pool debugging is enabled. Cheers, Stefan
