You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2011/02/08 03:08:41 UTC
svn commit: r1068249 - in /subversion/trunk/subversion:
libsvn_fs_util/caching.c libsvn_subr/cache-membuffer.c
Author: stefan2
Date: Tue Feb 8 02:08:40 2011
New Revision: 1068249
URL: http://svn.apache.org/viewvc?rev=1068249&view=rev
Log:
LINUX and possibly other OS will over-commit memory. I.e. actual
memory pages allocation will be deferred to the time of the first
access. As a result, the application may simply segfault later.
Therefore, ensure that the membuffer creation actually allocates
memory pages. Also, free the whole cache memory if allocation
of some parts failed. Continue with the cache disabled in that case.
* subversion/libsvn_fs_util/caching.c
(svn_fs__get_global_membuffer_cache): if cache creation fails,
actually free the previously allocated memory
* subversion/libsvn_subr/cache-membuffer.c
(secure_aligned_pcalloc): replacement for apr_pcalloc that will
not crash upon OOM
(svn_cache__membuffer_cache_create): ensure that memory
gets actually allocated despite of overcommitment by the OS;
don't try to continue upon OOM
Modified:
subversion/trunk/subversion/libsvn_fs_util/caching.c
subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Modified: subversion/trunk/subversion/libsvn_fs_util/caching.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_util/caching.c?rev=1068249&r1=1068248&r2=1068249&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_util/caching.c (original)
+++ subversion/trunk/subversion/libsvn_fs_util/caching.c Tue Feb 8 02:08:40 2011
@@ -96,10 +96,12 @@ svn_fs__get_global_membuffer_cache(void)
apr_uint64_t cache_size = cache_settings.cache_size;
if (!cache && cache_size)
{
+ svn_error_t *err;
+
svn_membuffer_t *old_cache = NULL;
svn_membuffer_t *new_cache = NULL;
- /* auto-allocate cache*/
+ /* auto-allocate cache */
apr_allocator_t *allocator = NULL;
apr_pool_t *pool = NULL;
@@ -111,16 +113,49 @@ svn_fs__get_global_membuffer_cache(void)
* in its full size, the create() function will clear the pool
* explicitly. The allocator will make sure that any memory no
* longer used by the pool will actually be returned to the OS.
+ *
+ * Please note that this pool and allocator is used *only* to
+ * allocate the large membuffer. All later dynamic allocations
+ * come from other, temporary pools and allocators.
*/
apr_allocator_max_free_set(allocator, 1);
- pool = svn_pool_create_ex(NULL, allocator);
- svn_error_clear(svn_cache__membuffer_cache_create(
+ /* don't terminate upon OOM but make pool return a NULL pointer
+ * instead so we can disable caching gracefully and continue
+ * operation without membuffer caches.
+ */
+ apr_pool_create_ex(&pool, NULL, NULL, allocator);
+ if (pool == NULL)
+ return NULL;
+
+ err = svn_cache__membuffer_cache_create(
&new_cache,
(apr_size_t)cache_size,
(apr_size_t)(cache_size / 16),
! svn_fs_get_cache_config()->single_threaded,
- pool));
+ pool);
+
+ /* Some error occured. Most likely it's an OOM error but we don't
+ * really care. Simply release all cache memory and disable caching
+ */
+ if (err)
+ {
+ /* Memory and error cleanup */
+ svn_error_clear(err);
+ apr_pool_destroy(pool);
+
+ /* Prevent future attempts to create the cache. However, an
+ * existing cache instance (see next comment) remains valid.
+ */
+ cache_settings.cache_size = 0;
+
+ /* The current caller won't get the cache object.
+ * However, a concurrent call might have succeeded in creating
+ * the cache object. That call and all following ones will then
+ * use the successfully created cache instance.
+ */
+ return NULL;
+ }
/* Handle race condition: if we are the first to create a
* cache object, make it our global singleton. Otherwise,
Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1068249&r1=1068248&r2=1068249&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Tue Feb 8 02:08:40 2011
@@ -712,6 +712,24 @@ ensure_data_insertable(svn_membuffer_t *
return FALSE;
}
+/* Mimic apr_pcalloc in APR_POOL_DEBUG mode, i.e. handle failed allocations
+ * (e.g. OOM) properly: Allocate at least SIZE bytes from POOL and zero
+ * the content of the allocated memory. If the allocation fails, return NULL.
+ *
+ * Also, satisfy our buffer alignment needs for performance reasons.
+ */
+static void* secure_aligned_pcalloc(apr_pool_t *pool, apr_size_t size)
+{
+ char* memory = apr_palloc(pool, (apr_size_t)size + ITEM_ALIGNMENT);
+ if (memory != NULL)
+ {
+ memory = (char *)ALIGN_POINTER(memory);
+ memset(memory, 0, size);
+ }
+
+ return memory;
+}
+
/* Create a new membuffer cache instance. If the TOTAL_SIZE of the
* memory i too small to accomodate the DICTIONARY_SIZE, the latte
* will be resized automatically. Also, a minumum size is assured
@@ -729,16 +747,11 @@ svn_cache__membuffer_cache_create(svn_me
apr_pool_t *pool)
{
/* allocate cache as an array of segments / cache objects */
- svn_membuffer_t *c = apr_palloc(pool, CACHE_SEGMENTS * sizeof(*c));
+ svn_membuffer_t *c = apr_pcalloc(pool, CACHE_SEGMENTS * sizeof(*c));
apr_uint32_t seg;
apr_uint32_t group_count;
apr_uint64_t data_size;
- /* We use this sub-pool to allocate the data buffer and the dictionary
- * so that we can release this memory easily upon OOM.
- */
- apr_pool_t *sub_pool = svn_pool_create(pool);
-
/* Split total cache size into segments of equal size
*/
total_size /= CACHE_SEGMENTS;
@@ -779,15 +792,13 @@ svn_cache__membuffer_cache_create(svn_me
*/
c[seg].group_count = group_count;
c[seg].directory =
- apr_palloc(sub_pool, group_count * sizeof(entry_group_t));
+ secure_aligned_pcalloc(pool, group_count * sizeof(entry_group_t));
c[seg].first = NO_INDEX;
c[seg].last = NO_INDEX;
c[seg].next = NO_INDEX;
c[seg].data_size = data_size;
- c[seg].data = apr_palloc(sub_pool, (apr_size_t)data_size);
- c[seg].data = (unsigned char *)ALIGN_POINTER(c[seg].data);
- c[seg].data_size -= ITEM_ALIGNMENT;
+ c[seg].data = secure_aligned_pcalloc(pool, (apr_size_t)data_size);
c[seg].current_data = 0;
c[seg].data_used = 0;
@@ -802,21 +813,9 @@ svn_cache__membuffer_cache_create(svn_me
*/
if (c[seg].data == NULL || c[seg].directory == NULL)
{
- /* in case we successfully allocated one part of the cache
- * make sure we release it asap.
- */
- svn_pool_destroy(sub_pool);
-
- c[seg].group_count = 1;
- c[seg].data_size = 0;
-
- if (c[seg].directory == NULL)
- c[seg].directory = apr_palloc(pool, sizeof(entry_group_t));
-
- /* if that modest allocation failed as well, we definitly are OOM.
+ /* We are OOM. There is no need to proceed with "half a cache".
*/
- if (c[seg].directory == NULL)
- return svn_error_wrap_apr(APR_ENOMEM, _("OOM"));
+ return svn_error_wrap_apr(APR_ENOMEM, _("OOM"));
}
/* initialize directory entries as "unused"
Re: svn commit: r1068249 - in /subversion/trunk/subversion:
libsvn_fs_util/caching.c libsvn_subr/cache-membuffer.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stefan2@apache.org wrote on Tue, Feb 08, 2011 at 02:08:41 -0000:
> @@ -111,16 +113,49 @@ svn_fs__get_global_membuffer_cache(void)
> + /* don't terminate upon OOM but make pool return a NULL pointer
> + * instead so we can disable caching gracefully and continue
> + * operation without membuffer caches.
> + */
Log an error?
(e.g., fs->warning_func())
> @@ -802,21 +813,9 @@ svn_cache__membuffer_cache_create(svn_me
> + return svn_error_wrap_apr(APR_ENOMEM, _("OOM"));
Spell out "Out of memory"?
Re: svn commit: r1068249 - in /subversion/trunk/subversion:
libsvn_fs_util/caching.c libsvn_subr/cache-membuffer.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stefan2@apache.org wrote on Tue, Feb 08, 2011 at 02:08:41 -0000:
> @@ -111,16 +113,49 @@ svn_fs__get_global_membuffer_cache(void)
> + /* don't terminate upon OOM but make pool return a NULL pointer
> + * instead so we can disable caching gracefully and continue
> + * operation without membuffer caches.
> + */
Log an error?
(e.g., fs->warning_func())
> @@ -802,21 +813,9 @@ svn_cache__membuffer_cache_create(svn_me
> + return svn_error_wrap_apr(APR_ENOMEM, _("OOM"));
Spell out "Out of memory"?