On Thu, Nov 13, 2008 at 02:03:39PM -0800, Jeremy Allison wrote: > On Thu, Nov 13, 2008 at 02:24:46PM +0100, Martin Zielinski wrote: > > Hi Volker, all! > > > > Yesterday I reported a memleak in the smbd on the technical mailing list > > with an attached pool-usage dump. > > Actually I'm not sure it was a 'leak' technically, it may > have been a too long delayed free from the talloc pool > (as no memory in a talloc pool is ever 'leaked' as such, > we may just neglect to free the talloc pool until smbd > exit :-).
Well, I'd call this pretty close to a memory leak.... Attached find a patch to 3.2 that might be an alternative to your change to memcache.c in 8962be69c700. What do you think? Martin, thanks a lot for reviewing our stuff! Volker
From 90b4a043bd5d0838ffb6aaad5413abe8c95d7071 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <[EMAIL PROTECTED]>
Date: Thu, 13 Nov 2008 23:50:19 +0100
Subject: [PATCH] Actually finish memcache_add_talloc
This fixes a memleak found by Martin Zielinski <[EMAIL PROTECTED]>. Thanks for
looking closely!
Volker
---
source/lib/memcache.c | 19 ++++++++++++++++++-
source/torture/torture.c | 33 ++++++++++++++++++++++++++++++++-
2 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/source/lib/memcache.c b/source/lib/memcache.c
index e1426bc..8ff75c4 100644
--- a/source/lib/memcache.c
+++ b/source/lib/memcache.c
@@ -214,6 +214,16 @@ static void memcache_delete_element(struct memcache *cache,
}
DLIST_REMOVE(cache->mru, e);
+ if (memcache_is_talloc(e->n)) {
+ DATA_BLOB cache_key, cache_value;
+ void *ptr;
+
+ memcache_element_parse(e, &cache_key, &cache_value);
+ SMB_ASSERT(cache_value.length == sizeof(ptr));
+ memcpy(&ptr, cache_value.data, sizeof(ptr));
+ TALLOC_FREE(ptr);
+ }
+
cache->size -= memcache_element_size(e->keylength, e->valuelength);
SAFE_FREE(e);
@@ -276,6 +286,12 @@ void memcache_add(struct memcache *cache, enum memcache_number n,
memcache_element_parse(e, &cache_key, &cache_value);
if (value.length <= cache_value.length) {
+ if (memcache_is_talloc(e->n)) {
+ void *ptr;
+ SMB_ASSERT(cache_value.length == sizeof(ptr));
+ memcpy(&ptr, cache_value.data, sizeof(ptr));
+ TALLOC_FREE(ptr);
+ }
/*
* We can reuse the existing record
*/
@@ -334,7 +350,8 @@ void memcache_add(struct memcache *cache, enum memcache_number n,
void memcache_add_talloc(struct memcache *cache, enum memcache_number n,
DATA_BLOB key, void *ptr)
{
- memcache_add(cache, n, key, data_blob_const(&ptr, sizeof(ptr)));
+ void *p = talloc_move(cache, &ptr);
+ memcache_add(cache, n, key, data_blob_const(&p, sizeof(p)));
}
void memcache_flush(struct memcache *cache, enum memcache_number n)
diff --git a/source/torture/torture.c b/source/torture/torture.c
index e909f8c..8419be7 100644
--- a/source/torture/torture.c
+++ b/source/torture/torture.c
@@ -5188,6 +5188,11 @@ static bool run_local_memcache(int dummy)
DATA_BLOB d1, d2, d3;
DATA_BLOB v1, v2, v3;
+ TALLOC_CTX *mem_ctx;
+ char *str1, *str2;
+ size_t size1, size2;
+ bool ret = false;
+
cache = memcache_init(NULL, 100);
if (cache == NULL) {
@@ -5239,7 +5244,33 @@ static bool run_local_memcache(int dummy)
}
TALLOC_FREE(cache);
- return true;
+
+ cache = memcache_init(NULL, 0);
+
+ mem_ctx = talloc_init("foo");
+
+ str1 = talloc_strdup(mem_ctx, "string1");
+ str2 = talloc_strdup(mem_ctx, "string2");
+
+ memcache_add_talloc(cache, SINGLETON_CACHE_TALLOC,
+ data_blob_string_const("torture"), str1);
+ size1 = talloc_total_size(cache);
+
+ memcache_add_talloc(cache, SINGLETON_CACHE_TALLOC,
+ data_blob_string_const("torture"), str2);
+ size2 = talloc_total_size(cache);
+
+ printf("size1=%d, size2=%d\n", (int)size1, (int)size2);
+
+ if (size2 > size1) {
+ printf("memcache leaks memory!\n");
+ goto fail;
+ }
+
+ ret = true;
+ fail:
+ TALLOC_FREE(cache);
+ return ret;
}
static double create_procs(bool (*fn)(int), bool *result)
--
1.5.3.6
pgpkd8M7cTajI.pgp
Description: PGP signature
-- To unsubscribe from this list go to the following URL and read the instructions: https://lists.samba.org/mailman/listinfo/samba
