On 17.01.2019 12:36, Yann Ylavic wrote:
> On Thu, Jan 17, 2019 at 12:10 PM Stefan Sperling <s...@stsp.name> 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 <s...@stsp.name> 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_t max_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_t max_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
