r15824: fixed a subtle talloc bug to do with memory context loops. When you

have a structure that references one of its parents, and a parent of
that parent is freed, then the whole structure should be freed, not
just the reference.

this was found by the change notify code, as a side effect of fixing
the memory leak yesterday
(This used to be commit 70531dcaeeb9314d410baa0d285df6a265311541)

diff --git a/source4/lib/talloc/talloc.c b/source4/lib/talloc/talloc.c
index 8f046a02b866436b1ad77bba8ad617040cca2ba8..823ae4ffb3d4e3ec4f396113f7c4c173a43e90a4 100644 (file)
--- a/source4/lib/talloc/talloc.c
+++ b/source4/lib/talloc/talloc.c
@@ -542,7 +542,13 @@ int talloc_free(void *ptr)
        tc = talloc_chunk_from_ptr(ptr);
 
        if (tc->refs) {
+               int is_child;
+               struct talloc_reference_handle *handle = tc->refs;
+               is_child = talloc_is_parent(handle, handle->ptr);
                talloc_reference_destructor(tc->refs);
+               if (is_child) {
+                       return talloc_free(ptr);
+               }
                return -1;
        }
 
@@ -1197,7 +1203,9 @@ void *talloc_find_parent_byname(const void *context, const char *name)
                        return TC_PTR_FROM_CHUNK(tc);
                }
                while (tc && tc->prev) tc = tc->prev;
-               tc = tc->parent;
+               if (tc) {
+                       tc = tc->parent;
+               }
        }
        return NULL;
 }
@@ -1219,6 +1227,30 @@ void talloc_show_parents(const void *context, FILE *file)
        while (tc) {
                fprintf(file, "\t'%s'\n", talloc_get_name(TC_PTR_FROM_CHUNK(tc)));
                while (tc && tc->prev) tc = tc->prev;
-               tc = tc->parent;
+               if (tc) {
+                       tc = tc->parent;
+               }
+       }
+}
+
+/*
+  return 1 if ptr is a parent of context
+*/
+int talloc_is_parent(const void *context, const char *ptr)
+{
+       struct talloc_chunk *tc;
+
+       if (context == NULL) {
+               return 0;
+       }
+
+       tc = talloc_chunk_from_ptr(context);
+       while (tc) {
+               if (TC_PTR_FROM_CHUNK(tc) == ptr) return 1;
+               while (tc && tc->prev) tc = tc->prev;
+               if (tc) {
+                       tc = tc->parent;
+               }
        }
+       return 0;
 }

diff --git a/source4/lib/talloc/talloc.h b/source4/lib/talloc/talloc.h
index 68bf24e0b8c216a825354b50a33d64f73b46ae98..99410241a6d6513ac4a026c20237a6f8f1da837d 100644 (file)
--- a/source4/lib/talloc/talloc.h
+++ b/source4/lib/talloc/talloc.h
@@ -137,6 +137,7 @@ void *talloc_autofree_context(void);
 size_t talloc_get_size(const void *ctx);
 void *talloc_find_parent_byname(const void *ctx, const char *name);
 void talloc_show_parents(const void *context, FILE *file);
+int talloc_is_parent(const void *context, const char *ptr);
 
 #endif
 

diff --git a/source4/lib/talloc/testsuite.c b/source4/lib/talloc/testsuite.c
index b03be98587aaf06a92f93bfcdac879e2f0fe94b2..018d734cc34309cb5b342279511ea84926245eed 100644 (file)
--- a/source4/lib/talloc/testsuite.c
+++ b/source4/lib/talloc/testsuite.c
@@ -824,7 +824,7 @@ static BOOL test_speed(void)
 }
 
 
-BOOL test_lifeless(void)
+static BOOL test_lifeless(void)
 {
        char *top = talloc_new(NULL);
        char *parent, *child; 
@@ -836,10 +836,53 @@ BOOL test_lifeless(void)
        child = talloc_strdup(parent, "child");  
        talloc_reference(child, parent);
        talloc_reference(child_owner, child); 
+       talloc_report_full(top, stdout);
        talloc_unlink(top, parent);
        talloc_free(child);
        talloc_report_full(top, stdout);
        talloc_free(top);
+       talloc_free(child_owner);
+#if 0
+       talloc_free(child);
+#endif
+       return True;
+}
+
+static int loop_destructor_count;
+
+static int test_loop_destructor(void *ptr)
+{
+       printf("loop destructor\n");
+       loop_destructor_count++;
+       return 0;
+}
+
+static BOOL test_loop(void)
+{
+       char *top = talloc_new(NULL);
+       char *parent;
+       struct req1 {
+               char *req2, *req3;
+       } *req1;
+
+       printf("TESTING TALLOC LOOP DESTRUCTION\n");
+       parent = talloc_strdup(top, "parent");
+       req1 = talloc(parent, struct req1);
+       req1->req2 = talloc_strdup(req1, "req2");  
+       talloc_set_destructor(req1->req2, test_loop_destructor);
+       req1->req3 = talloc_strdup(req1, "req3");
+       talloc_reference(req1->req3, req1);
+       talloc_report_full(top, stdout);
+       talloc_free(parent);
+       talloc_report_full(top, stdout);
+       talloc_report_full(NULL, stdout);
+       talloc_free(top);
+
+       if (loop_destructor_count != 1) {
+               printf("FAILED TO FIRE LOOP DESTRUCTOR\n");
+               return False;
+       }
+
        return True;
 }
 
@@ -861,6 +904,7 @@ BOOL torture_local_talloc(struct torture_context *torture)
        ret &= test_realloc_fn();
        ret &= test_type();
        ret &= test_lifeless();
+       ret &= test_loop();
        if (ret) {
                ret &= test_speed();
        }