stef...@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...
