stefan2_at_apache.org wrote on Tue, Nov 09, 2010 at 15:51:54 -0000:
> Author: stefan2
> Date: Tue Nov 9 15:51:54 2010
> New Revision: 1033040
>
> URL: http://svn.apache.org/viewvc?rev=1033040&view=rev
> Log:
> Fix error leaks in cache creation code. Also, having a NULL
> file handle cache is not allowed (it is only allowed for the
> membuffer cache).
>
> * subversion/libsvn_fs_util/caching.c
> (svn_fs__get_global_membuffer_cache): fix error leak
> (svn_fs__get_global_file_handle_cache): prevent PFs later by exiting
> the process immediately upon cache creation failure
>
> Suggested by: stsp
>
> Modified:
> subversion/branches/performance/subversion/libsvn_fs_util/caching.c
>
> Modified: subversion/branches/performance/subversion/libsvn_fs_util/caching.c
> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_fs_util/caching.c?rev=1033040&r1=1033039&r2=1033040&view=diff
> ==============================================================================
> --- subversion/branches/performance/subversion/libsvn_fs_util/caching.c (original)
> +++ subversion/branches/performance/subversion/libsvn_fs_util/caching.c Tue Nov 9 15:51:54 2010
> @@ -49,7 +49,8 @@ svn_fs_get_cache_config(void)
>
> /* Access the process-global (singleton) membuffer cache. The first call
> * will automatically allocate the cache using the current cache config.
> - * NULL will be returned if the desired cache size is 0.
> + * NULL will be returned if the desired cache size is 0 or if the cache
> + * could not be created for some reason.
> */
> svn_membuffer_t *
> svn_fs__get_global_membuffer_cache(void)
> @@ -75,12 +76,12 @@ svn_fs__get_global_membuffer_cache(void)
> apr_allocator_max_free_set(allocator, 1);
> pool = svn_pool_create_ex(NULL, allocator);
>
> - svn_cache__membuffer_cache_create
> - (&cache,
> - (apr_size_t)cache_size,
> - (apr_size_t)(cache_size / 16),
> - ! svn_fs_get_cache_config()->single_threaded,
> - pool);
> + svn_error_clear(svn_cache__membuffer_cache_create(
> + &cache,
> + (apr_size_t)cache_size,
> + (apr_size_t)(cache_size / 16),
> + ! svn_fs_get_cache_config()->single_threaded,
> + pool));
> }
>
> return cache;
> @@ -96,10 +97,23 @@ svn_fs__get_global_file_handle_cache(voi
> static svn_file_handle_cache_t *cache = NULL;
>
> if (!cache)
> - svn_file_handle_cache__create_cache(&cache,
> - cache_settings.file_handle_count,
> - !cache_settings.single_threaded,
> - svn_pool_create(NULL));
> + {
> + svn_error_t* err = svn_file_handle_cache__create_cache(
> + &cache,
> + cache_settings.file_handle_count,
> + !cache_settings.single_threaded,
> + svn_pool_create(NULL));
> + if (err)
> + {
> + svn_error_clear(err);
> +
> + /* We need the file handle cache. The only way that an error could
> + * occur would be some threading error. In that case, there is no
> + * way we could continue - not even in some limp home mode.
> + */
> + SVN_ERR_MALFUNCTION_NO_RETURN();
Haven't looked at it closely, but:
It seems a shame to lose ERR here. Could we (better) pass it to the
malfunction handler, or (at least) log it to the FS warning function?
> + }
> + }
>
> return cache;
> }
>
>
BTW, what's the status of the performance branch? Last I check, its
STATUS File was empty...
Received on 2010-11-09 23:40:59 CET