Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Date: Sat, 11 Feb 2017 17:18:29 +0100
From: Stefan Sperling <stsp@apache.org>
To: dev@subversion.apache.org
Cc: stefan2@apache.org
Subject: why svn object pools? (Re: svn commit: r1782614 -
 /subversion/trunk/subversion/libsvn_repos/authz.c)
Message-ID: <20170211161828.suwsjqdr7kk2q624@jim.stsp.name>
Mail-Followup-To: dev@subversion.apache.org, stefan2@apache.org
References: <20170211160707.A7E733A0069@svn01-us-west.apache.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170211160707.A7E733A0069@svn01-us-west.apache.org>
User-Agent: NeoMutt/20170128 (1.7.2)
archived-at: Sat, 11 Feb 2017 17:13:39 -0000

On Sat, Feb 11, 2017 at 04:07:07PM -0000, stsp@apache.org wrote:
> Author: stsp
> Date: Sat Feb 11 16:07:06 2017
> New Revision: 1782614
> 
> URL: http://svn.apache.org/viewvc?rev=1782614&view=rev
> Log:
> Fix a crash during init in libsvn_repos with pool debugging enabled.
> 
> * subversion/libsvn_repos/authz.c
>   (synchronized_authz_initialize): With APR_POOL_DEBUG, the function
>    apr_pool_allocator_get() will return NULL (there is no global allocator).
>    So dereferencing that pointer to find the allocator's mutex will segfault.
>    It is unclear to me if using apr_allocator_mutex_get() is a sane way to find
>    out if we're running in multithreaded mode, and I can't find a better way
>    right now. For now, just assume that we're multithreaded if APR has threads.
>    This should be safe and allows me to run the tests over DAV again.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_repos/authz.c
> 
> Modified: subversion/trunk/subversion/libsvn_repos/authz.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/authz.c?rev=1782614&r1=1782613&r2=1782614&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/authz.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/authz.c Sat Feb 11 16:07:06 2017
> @@ -135,8 +135,7 @@ static svn_error_t *
>  synchronized_authz_initialize(void *baton, apr_pool_t *pool)
>  {
>  #if APR_HAS_THREADS
> -  svn_boolean_t multi_threaded
> -    = apr_allocator_mutex_get(apr_pool_allocator_get(pool)) != NULL;
> +  svn_boolean_t multi_threaded = TRUE;
>  #else
>    svn_boolean_t multi_threaded = FALSE;
>  #endif
> 

The reason the above code wants to know if we're using threads is that
there is a new private svn_object_pool API which requires this information.

Is this new API critical to the new authz implementation?
It seems like yet another layer which adds complexity and maintenance burden.
Could the new authz implementation work without this? What would the impact be?
Why aren't plain old APR pools good enough?