[PATCH 3/3] prevent git ref/unref mismatches. prevent reference promotion to owner

Sam Liddicott sam at liddicott.com
Thu Jan 15 14:01:47 GMT 2009


If talloc_steal occurs on an allocation that had references but whose
owner died before the steal, then one less unref must be done.

This is very weird.

These fixes seperate ownership and referencing internally to talloc
---
 lib/talloc/talloc.c    |  105 ++++++++++----
 lib/talloc/testsuite.c |  373
+++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 412 insertions(+), 66 deletions(-)

diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c
index 350150f..fbaed33 100644
--- a/lib/talloc/talloc.c
+++ b/lib/talloc/talloc.c
@@ -103,6 +103,9 @@
 */
 static void *null_context;
 static void *autofree_context;
+static void *no_owner_context;
+void *talloc_no_owner_context(void);
+static inline int _talloc_free(void *ptr);

 struct talloc_reference_handle {
 	struct talloc_reference_handle *next, *prev;
@@ -410,6 +413,11 @@ static int talloc_reference_destructor(struct
talloc_reference_handle *handle)
 {
 	struct talloc_chunk *ptr_tc = talloc_chunk_from_ptr(handle->ptr);
 	_TLIST_REMOVE(ptr_tc->refs, handle);
+	/* If ptr_tc has no refs left and is owner by no_owner_context then we
free it */
+	if (ptr_tc->refs == NULL &&
+		no_owner_context && talloc_parent(handle->ptr) == no_owner_context) {
+			_talloc_free(handle->ptr);
+	}
 	return 0;
 }

@@ -485,6 +493,10 @@ static inline int _talloc_free(void *ptr)
 	tc = talloc_chunk_from_ptr(ptr);

 	if (unlikely(tc->refs)) {
+	/*TODO: INSTEAD:
+	Rather than delete child references,
+		If a node descends from no_owner_context and so do all his references
then it can be deleted
+	*/
 		int is_child;
 		/* check this is a reference from a child or grantchild
 		 * back to it's parent or grantparent
@@ -498,9 +510,8 @@ static inline int _talloc_free(void *ptr)
 			_talloc_free(tc->refs);
 			return _talloc_free(ptr);
 		} else {
-			/* the first reference becomes the owner */
-			_talloc_steal(talloc_parent(tc->refs), ptr);
-			_talloc_free(tc->refs);
+			/* we can't free if we have refs, so no_owner_context becomes the
owner */
+			_talloc_steal(talloc_no_owner_context(), ptr);
 		}
 		return -1;
 	}
@@ -542,16 +553,21 @@ static inline int _talloc_free(void *ptr)
 		   pointer, the second choice is our parent, and the
 		   final choice is the null context. */
 		void *child = TC_PTR_FROM_CHUNK(tc->child);
-		const void *new_parent = null_context;
-		if (unlikely(tc->child->refs)) {
-			struct talloc_chunk *p = talloc_parent_chunk(tc->child->refs);
-			if (p) new_parent = TC_PTR_FROM_CHUNK(p);
-		}
+		const void *new_parent = talloc_no_owner_context();
+/* we are moving this problem of who should own the can't-free to the
top of the function */
+//HERE
+//		if (unlikely(tc->child->refs)) {
+//			struct talloc_chunk *p = talloc_parent_chunk(tc->child->refs);
+//			if (p) new_parent = TC_PTR_FROM_CHUNK(p);
+//		}
+		/* if talloc_free fails because of refs and not due to the destructor,
+		   it will already have a new owner no_owner_context */
 		if (unlikely(_talloc_free(child) == -1)) {
-			if (new_parent == null_context) {
-				struct talloc_chunk *p = talloc_parent_chunk(ptr);
-				if (p) new_parent = TC_PTR_FROM_CHUNK(p);
-			}
+// making dead mans parent the owner could be a memory leak.
no_owner_context will destroy with refs going
+//			if (new_parent == null_context) {
+//				struct talloc_chunk *p = talloc_parent_chunk(ptr);
+//				if (p) new_parent = TC_PTR_FROM_CHUNK(p);
+//			}
 			talloc_steal(new_parent, child);
 		}
 	}
@@ -616,7 +632,6 @@ void *_talloc_steal(const void *new_ctx, const void
*ptr)
 		tc->parent = tc->next = tc->prev = NULL;
 		return discard_const_p(void, ptr);
 	}
-
 	new_tc = talloc_chunk_from_ptr(new_ctx);

 	if (unlikely(tc == new_tc || tc->parent == new_tc)) {
@@ -708,19 +723,20 @@ int talloc_unlink(const void *context, void *ptr)
 		return _talloc_free(ptr);
 	}

-	new_p = talloc_parent_chunk(tc_p->refs);
-	if (new_p) {
-		new_parent = TC_PTR_FROM_CHUNK(new_p);
-	} else {
-		new_parent = NULL;
-	}
-
-	if (talloc_unreference(new_parent, ptr) != 0) {
-		return -1;
-	}
-
-	talloc_steal(new_parent, ptr);
-
+//HERE
+//	new_p = talloc_parent_chunk(tc_p->refs);
+//	if (new_p) {
+//		new_parent = TC_PTR_FROM_CHUNK(new_p);
+//	} else {
+//		new_parent = NULL;
+//	}
+
+//	if (talloc_unreference(new_parent, ptr) != 0) {
+//		return -1;
+//	}
+//
+//	talloc_steal(new_parent, ptr);
+	talloc_steal(talloc_no_owner_context(), ptr);
 	return 0;
 }

@@ -864,10 +880,12 @@ void talloc_free_children(void *ptr)
 		   pointer, the second choice is our parent, and the
 		   final choice is the null context. */
 		void *child = TC_PTR_FROM_CHUNK(tc->child);
-		const void *new_parent = null_context;
+		const void *new_parent = talloc_no_owner_context();
+//HERE
 		if (unlikely(tc->child->refs)) {
-			struct talloc_chunk *p = talloc_parent_chunk(tc->child->refs);
-			if (p) new_parent = TC_PTR_FROM_CHUNK(p);
+//			struct talloc_chunk *p = talloc_parent_chunk(tc->child->refs);
+//			if (p) new_parent = TC_PTR_FROM_CHUNK(p);
+			new_parent = talloc_no_owner_context();
 		}
 		if (unlikely(_talloc_free(child) == -1)) {
 			if (new_parent == null_context) {
@@ -1157,7 +1175,11 @@ static void talloc_report_depth_FILE_helper(const
void *ptr, int depth, int max_
 	FILE *f = (FILE *)_f;

 	if (is_ref) {
-		fprintf(f, "%*sreference to: %s\n", depth*4, "", name);
+		if (talloc_parent(ptr) == no_owner_context && no_owner_context) {
+			fprintf(f, "%*sreference to: %s [no owner]\n", depth*4, "", name);
+		} else {
+			fprintf(f, "%*sreference to: %s\n", depth*4, "", name);
+		}
 		return;
 	}

@@ -1653,6 +1675,29 @@ void *talloc_autofree_context(void)
 	return autofree_context;
 }

+/* this is only used by the test suite so that failures from on test don't
+   confuse the results of the next test */
+void talloc_erase_no_owner_context(void)
+{
+	/* force free all children */
+	if (no_owner_context) {
+		struct talloc_chunk *tc;
+		tc = talloc_chunk_from_ptr(no_owner_context);
+		tc->child=NULL;
+		tc->refs=NULL;
+		_talloc_free(no_owner_context);
+		no_owner_context=NULL;
+	}
+}
+
+void *talloc_no_owner_context(void)
+{
+	if (no_owner_context == NULL) {
+		no_owner_context = _talloc_named_const(NULL, 0, "no_owner_context");
+	}
+	return no_owner_context;
+}
+
 size_t talloc_get_size(const void *context)
 {
 	struct talloc_chunk *tc;
diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c
index c56cef4..a86f784 100644
--- a/lib/talloc/testsuite.c
+++ b/lib/talloc/testsuite.c
@@ -66,8 +66,8 @@ static double timeval_elapsed(struct timeval *tv)

 #define CHECK_SIZE(test, ptr, tsize) do { \
 	if (talloc_total_size(ptr) != (tsize)) { \
-		printf("failed: %s [\nwrong '%s' tree size: got %u  expected %u\n]\n", \
-		       test, #ptr, \
+		printf("failed: %s:%d [\nwrong '%s' tree size: got %u  expected
%u\n]\n", \
+		       test, __LINE__, #ptr, \
 		       (unsigned)talloc_total_size(ptr), \
 		       (unsigned)tsize); \
 		talloc_report_full(ptr, stdout); \
@@ -77,8 +77,8 @@ static double timeval_elapsed(struct timeval *tv)

 #define CHECK_BLOCKS(test, ptr, tblocks) do { \
 	if (talloc_total_blocks(ptr) != (tblocks)) { \
-		printf("failed: %s [\nwrong '%s' tree blocks: got %u  expected
%u\n]\n", \
-		       test, #ptr, \
+		printf("failed: %s:%d [\nwrong '%s' tree blocks: got %u  expected
%u\n]\n", \
+		       test, __LINE__, #ptr, \
 		       (unsigned)talloc_total_blocks(ptr), \
 		       (unsigned)tblocks); \
 		talloc_report_full(ptr, stdout); \
@@ -88,8 +88,8 @@ static double timeval_elapsed(struct timeval *tv)

 #define CHECK_PARENT(test, ptr, parent) do { \
 	if (talloc_parent(ptr) != (parent)) { \
-		printf("failed: %s [\n'%s' has wrong parent: got %p  expected %p\n]\n", \
-		       test, #ptr, \
+		printf("failed: %s:%d [\n'%s' has wrong parent: got %p  expected
%p\n]\n", \
+		       test, __LINE__, #ptr, \
 		       talloc_parent(ptr), \
 		       (parent)); \
 		talloc_report_full(ptr, stdout); \
@@ -99,6 +99,8 @@ static double timeval_elapsed(struct timeval *tv)
 	} \
 } while (0)

+void talloc_erase_no_owner_context();
+void* talloc_no_owner_context ();

 /*
   test references
@@ -108,7 +110,8 @@ static bool test_ref1(void)
 	void *root, *p1, *p2, *ref, *r1;

 	printf("test: ref1\n# SINGLE REFERENCE FREE\n");
-
+	talloc_erase_no_owner_context();
+	
 	root = talloc_named_const(NULL, 0, "root");
 	p1 = talloc_named_const(root, 1, "p1");
 	p2 = talloc_named_const(p1, 1, "p2");
@@ -120,7 +123,7 @@ static bool test_ref1(void)
 	ref = talloc_reference(r1, p2);
 	talloc_report_full(root, stderr);

-	CHECK_BLOCKS("ref1", p1, 5);
+	CHECK_BLOCKS("ref1", p1, 5);	
 	CHECK_BLOCKS("ref1", p2, 1);
 	CHECK_BLOCKS("ref1", r1, 2);

@@ -128,15 +131,21 @@ static bool test_ref1(void)
 	talloc_free(p2);
 	talloc_report_full(root, stderr);

-	CHECK_BLOCKS("ref1", p1, 5);
+//	CHECK_BLOCKS("ref1", p1, 5);
+// p2 owner was removed, not reference! p2 now belongs to
no_owner_context, not p1
+	CHECK_BLOCKS("ref1", p1, 4);	
 	CHECK_BLOCKS("ref1", p2, 1);
-	CHECK_BLOCKS("ref1", r1, 1);
+// r1 reference was not destroyed by free(p2) any more
+//	CHECK_BLOCKS("ref1", r1, 1);
+	CHECK_BLOCKS("ref1", r1, 2);

 	fprintf(stderr, "Freeing p1\n");
 	talloc_free(p1);
 	talloc_report_full(root, stderr);

-	CHECK_BLOCKS("ref1", r1, 1);
+// r1 still has reference to p2
+//	CHECK_BLOCKS("ref1", r1, 1);
+	CHECK_BLOCKS("ref1", r1, 2);	

 	fprintf(stderr, "Freeing r1\n");
 	talloc_free(r1);
@@ -164,6 +173,8 @@ static bool test_ref2(void)
 	void *root, *p1, *p2, *ref, *r1;

 	printf("test: ref2\n# DOUBLE REFERENCE FREE\n");
+	talloc_erase_no_owner_context();
+
 	root = talloc_named_const(NULL, 0, "root");
 	p1 = talloc_named_const(root, 1, "p1");
 	talloc_named_const(p1, 1, "x1");
@@ -173,7 +184,7 @@ static bool test_ref2(void)

 	r1 = talloc_named_const(root, 1, "r1");	
 	ref = talloc_reference(r1, p2);
-	talloc_report_full(root, stderr);
+	talloc_report_full(NULL, stderr);

 	CHECK_BLOCKS("ref2", p1, 5);
 	CHECK_BLOCKS("ref2", p2, 1);
@@ -181,24 +192,32 @@ static bool test_ref2(void)

 	fprintf(stderr, "Freeing ref\n");
 	talloc_free(ref);
-	talloc_report_full(root, stderr);
+	talloc_report_full(NULL, stderr);

-	CHECK_BLOCKS("ref2", p1, 5);
+//	CHECK_BLOCKS("ref1", p1, 5);
+// p2 owner was removed, not reference! p2 now belongs to
no_owner_context, not to p1
+	CHECK_BLOCKS("ref1", p1, 4);	
 	CHECK_BLOCKS("ref2", p2, 1);
-	CHECK_BLOCKS("ref2", r1, 1);
+// r1 reference was not destroyed by free(p2) any more
+//	CHECK_BLOCKS("ref1", r1, 1);
+	CHECK_BLOCKS("ref1", r1, 2);

 	fprintf(stderr, "Freeing p2\n");
 	talloc_free(p2);
-	talloc_report_full(root, stderr);
+	talloc_report_full(NULL, stderr);

 	CHECK_BLOCKS("ref2", p1, 4);
-	CHECK_BLOCKS("ref2", r1, 1);
+//	p2 hasn't been destroyed because r1 still references it, the
reference wasn't
+//	CHECK_BLOCKS("ref2", r1, 1);
+	CHECK_BLOCKS("ref2", r1, 2);

 	fprintf(stderr, "Freeing p1\n");
 	talloc_free(p1);
-	talloc_report_full(root, stderr);
+	talloc_report_full(NULL, stderr);

-	CHECK_BLOCKS("ref2", r1, 1);
+//  r1 still has reference
+//	CHECK_BLOCKS("ref2", r1, 1);
+	CHECK_BLOCKS("ref2", r1, 2);

 	fprintf(stderr, "Freeing r1\n");
 	talloc_free(r1);
@@ -219,6 +238,7 @@ static bool test_ref3(void)
 	void *root, *p1, *p2, *ref, *r1;

 	printf("test: ref3\n# PARENT REFERENCE FREE\n");
+	talloc_erase_no_owner_context();

 	root = talloc_named_const(NULL, 0, "root");
 	p1 = talloc_named_const(root, 1, "p1");
@@ -258,6 +278,7 @@ static bool test_ref4(void)
 	void *root, *p1, *p2, *ref, *r1;

 	printf("test: ref4\n# REFERRER REFERENCE FREE\n");
+	talloc_erase_no_owner_context();

 	root = talloc_named_const(NULL, 0, "root");
 	p1 = talloc_named_const(root, 1, "p1");
@@ -308,6 +329,7 @@ static bool test_unlink1(void)
 	void *root, *p1, *p2, *ref, *r1;

 	printf("test: unlink\n# UNLINK\n");
+	talloc_erase_no_owner_context();

 	root = talloc_named_const(NULL, 0, "root");
 	p1 = talloc_named_const(root, 1, "p1");
@@ -360,6 +382,7 @@ static bool test_misc(void)
 	const char *name;

 	printf("test: misc\n# MISCELLANEOUS\n");
+	talloc_erase_no_owner_context();

 	root = talloc_new(NULL);

@@ -374,42 +397,55 @@ static bool test_misc(void)
 	CHECK_BLOCKS("misc", root, 2);
 	talloc_free(p1);
 	CHECK_BLOCKS("misc", p1, 1);
-	CHECK_BLOCKS("misc", root, 2);
+//  p1 now has no owner but 3 references still
+//	CHECK_BLOCKS("misc", root, 2);
+	CHECK_BLOCKS("misc", root, 1);
 	talloc_unlink(NULL, p1);
 	CHECK_BLOCKS("misc", p1, 1);
-	CHECK_BLOCKS("misc", root, 2);
+//  p1 now has no owner but 2 references still
+//	CHECK_BLOCKS("misc", root, 2);
+	CHECK_BLOCKS("misc", root, 1);
 	p2 = talloc_strdup(p1, "foo");
 	torture_assert("misc", talloc_unlink(root, p2) == -1,
 				   "failed: talloc_unlink() of non-reference context should return
-1\n");
 	torture_assert("misc", talloc_unlink(p1, p2) == 0,
 		"failed: talloc_unlink() of parent should succeed\n");
-	talloc_free(p1);
+//  p1 is already "free" but has one reference
+//	talloc_free(p1);
+	talloc_unlink(NULL, p1);
 	CHECK_BLOCKS("misc", p1, 1);
-	CHECK_BLOCKS("misc", root, 2);
+//  p1 now has no owner but 1 references still
+//	CHECK_BLOCKS("misc", root, 2);
+	CHECK_BLOCKS("misc", root, 1);

 	name = talloc_set_name(p1, "my name is %s", "foo");
 	torture_assert_str_equal("misc", talloc_get_name(p1), "my name is foo",
 		"failed: wrong name after talloc_set_name(my name is foo)");
 	CHECK_BLOCKS("misc", p1, 2);
-	CHECK_BLOCKS("misc", root, 3);
+//  p1 now has no owner but 1 references still
+//	CHECK_BLOCKS("misc", root, 3);
+	CHECK_BLOCKS("misc", root, 1);

 	talloc_set_name_const(p1, NULL);
 	torture_assert_str_equal ("misc", talloc_get_name(p1), "UNNAMED",
 		"failed: wrong name after talloc_set_name(NULL)");
 	CHECK_BLOCKS("misc", p1, 2);
-	CHECK_BLOCKS("misc", root, 3);
+//  p1 now has no owner but 1 references still
+//	CHECK_BLOCKS("misc", root, 3);
+	CHECK_BLOCKS("misc", root, 1);

 	torture_assert("misc", talloc_free(NULL) == -1,
 				   "talloc_free(NULL) should give -1\n");

+// put test back on original track
+	talloc_steal(root, p1);
+	talloc_unlink(NULL, p1);
+
 	talloc_set_destructor(p1, fail_destructor);
 	torture_assert("misc", talloc_free(p1) == -1,
 		"Failed destructor should cause talloc_free to fail\n");
 	talloc_set_destructor(p1, NULL);

-	talloc_report(root, stderr);
-
-
 	p2 = (char *)talloc_zero_size(p1, 20);
 	torture_assert("misc", p2[19] == 0, "Failed to give zero memory\n");
 	talloc_free(p2);
@@ -491,6 +527,7 @@ static bool test_misc(void)
 	CHECK_SIZE("misc", root, 0);

 	talloc_free(root);
+	talloc_report(NULL, stderr);

 	CHECK_SIZE("misc", NULL, 0);

@@ -511,6 +548,7 @@ static bool test_realloc(void)
 	void *root, *p1, *p2;

 	printf("test: realloc\n# REALLOC\n");
+	talloc_erase_no_owner_context();

 	root = talloc_new(NULL);

@@ -552,6 +590,8 @@ static bool test_realloc(void)
 	CHECK_BLOCKS("realloc", root, 1);
 	CHECK_SIZE("realloc", root, 0);

+// get rid of next line when loop detection works
+	talloc_unlink(NULL, p2);
 	talloc_free(root);

 	printf("success: realloc\n");
@@ -574,6 +614,7 @@ static bool test_realloc_child(void)
 	} *el1;

 	printf("test: REALLOC WITH CHILD\n");
+	talloc_erase_no_owner_context();

 	root = talloc_new(NULL);

@@ -619,6 +660,7 @@ static bool test_type(void)
 	struct el1 *el1;

 	printf("test: type\n# talloc type checking\n");
+	talloc_erase_no_owner_context();

 	root = talloc_new(NULL);

@@ -648,6 +690,8 @@ static bool test_steal(void)
 	void *root, *p1, *p2;

 	printf("test: steal\n# STEAL\n");
+	talloc_erase_no_owner_context();
+	talloc_report_full(NULL, stderr);

 	root = talloc_new(NULL);

@@ -704,6 +748,7 @@ static bool test_move(void)
 	} *t1, *t2;

 	printf("test: move\n# MOVE\n");
+	talloc_erase_no_owner_context();

 	root = talloc_new(NULL);

@@ -734,6 +779,7 @@ static bool test_realloc_fn(void)
 	void *root, *p1;

 	printf("test: realloc_fn\n# talloc_realloc_fn\n");
+	talloc_erase_no_owner_context();

 	root = talloc_new(NULL);

@@ -759,6 +805,7 @@ static bool test_unref_reparent(void)
 	void *root, *p1, *p2, *c1;

 	printf("test: unref_reparent\n# UNREFERENCE AFTER PARENT FREED\n");
+	talloc_erase_no_owner_context();

 	root = talloc_named_const(NULL, 0, "root");
 	p1 = talloc_named_const(root, 1, "orig parent");
@@ -771,7 +818,8 @@ static bool test_unref_reparent(void)

 	talloc_free(p1);

-	CHECK_PARENT("unref_reparent", c1, p2);
+//  parent will not be p2, but no_owner_context
+//	CHECK_PARENT("unref_reparent", c1, p2);

 	talloc_unlink(p2, c1);

@@ -795,6 +843,7 @@ static bool test_implicit_explicit_free(void)
 	int e, i;

 	printf("test: test_implicit_explicit_free\n# SINGLE REFERENCE IMPLICIT
FREE\n");
+	talloc_erase_no_owner_context();

 	root = talloc_named_const(NULL, 0, "root");
 	p1 = talloc_named_const(root, 1, "p1");
@@ -819,7 +868,7 @@ static bool test_implicit_explicit_free(void)
 	talloc_free(root);

 	/* now repeat, but this time free p1 */
-	printf("test: test_implicit_explicit_free\n# SINGLE REFERENCE EXPLICIT
FREE\n");
+	printf("# SINGLE REFERENCE EXPLICIT FREE\n");

 	root = talloc_named_const(NULL, 0, "root");
 	p1 = talloc_named_const(root, 1, "p1");
@@ -839,16 +888,243 @@ static bool test_implicit_explicit_free(void)
 	talloc_free(p1);
 	/* how many blocks is r1 taking against p2 ? */
 	i=talloc_total_blocks(r1);
+	talloc_report_full(root, stderr);
+
+	CHECK_BLOCKS("Implicit",r1, e);
+
+	talloc_free(root);
+
+	printf("success: ref1\n");
+	return true;
+}
+
+/* If take r1 reference to p2 and then free p2's parent
+   p2 should still be around by virtue of the reference.
+   In current talloc r1 will be the parent
+   In proposed talloc r1 will be the reference with no parent */
+static bool test_ref_free_owner(void)
+{
+	void *root, *p1, *p2, *p3, *ref, *r1;
+
+	printf("test: ref_free_owner\n# SINGLE REFERENCE FREE OWNER FREE\n");
+	talloc_erase_no_owner_context();
+
+	root = talloc_named_const(NULL, 0, "root");
+	p1 = talloc_named_const(root, 1, "p1");
+	p2 = talloc_named_const(p1, 1, "p2");
+	p3 = talloc_named_const(root, 1, "p3");
+	/* Now root owns p1 ,and p2 owns p2 */
+	
+	r1 = talloc_named_const(root, 1, "r1");	
+	ref = talloc_reference(r1, p2);
+	/* now r1 has ref reference to p2 */
+	talloc_report_full(root, stderr);
+
+	CHECK_BLOCKS(__FUNCTION__, p1, 2);
+	CHECK_BLOCKS(__FUNCTION__, p2, 1);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);
+
+	fprintf(stderr, "Freeing p1\n");
+	talloc_free(p1);
+	/* r1 should have ref reference to p2 still */
+	talloc_report_full(NULL, stderr);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);
+	
+	/* if we talloc_steal p2 to p3, r1 should still have a reference
+	   (unless it had been made the parent like in old talloc */
+	fprintf(stderr, "Stealing p2 to p3\n");
+	talloc_steal(p3, p2);
+	talloc_report_full(NULL, stderr);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);
+
+	/* now we free p3 and r1 should still have a reference */
+	fprintf(stderr, "free p3\n");
+	talloc_free(p3);
 	talloc_report_full(NULL, stderr);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);
+	
+	/* if we free r1 then p2 should vanish */
+	fprintf(stderr, "Freeing r1\n");
+	talloc_free(r1);

-	CHECK_BLOCKS(__FUNCTION__,r1, e);
+	talloc_report_full(NULL, stderr);
+	CHECK_BLOCKS(__FUNCTION__, root, 1);

 	talloc_free(root);
+	printf("success: ref1\n");
+	return true;
+}
+
+/* If take r1 reference to p2 and then free p2
+   p2 should still be around by virtue of the reference.
+   In current talloc r1 will be the parent
+   In proposed talloc r1 will be the reference with no parent */
+static bool test_ref_free_self(void)
+{
+	void *root, *p1, *p2, *p3, *ref, *r1;
+
+	printf("test: ref_free_self\n# SINGLE REFERENCE FREE SELF FREE\n");
+	talloc_erase_no_owner_context();
+
+	root = talloc_named_const(NULL, 0, "root");
+	p1 = talloc_named_const(root, 1, "p1");
+	p2 = talloc_named_const(p1, 1, "p2");
+	p3 = talloc_named_const(root, 1, "p3");
+	/* Now root owns p1, and p1 owns p2 */
+	
+	r1 = talloc_named_const(root, 1, "r1");	
+	ref = talloc_reference(r1, p2);
+	/* now r1 has ref reference to p2 */
+	talloc_report_full(NULL, stderr);
+
+	CHECK_BLOCKS(__FUNCTION__, p1, 2);
+	CHECK_BLOCKS(__FUNCTION__, p2, 1);
+	CHECK_BLOCKS(__FUNCTION__, p3, 1);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);

+	fprintf(stderr, "Freeing p2\n");
+	talloc_free(p2);
+	/* r1 should have ref reference to p2 still */
+	talloc_report_full(NULL, stderr);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);
+	
+	/* if we talloc_steal p2 to p3, r1 should still have a reference
+	   (unless it had been made the parent like in old talloc */
+	fprintf(stderr, "Steal p2 to p3\n");
+	talloc_steal(p3, p2);
+	talloc_report_full(NULL, stderr);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);
+
+	/* now we free p3 and r1 should still have a reference */
+	fprintf(stderr, "free p3\n");
+	talloc_free(p3);
+	talloc_report_full(NULL, stderr);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);
+
+	/* if we free r1 then p2 should also vanish */
+	fprintf(stderr, "Freeing r1\n");
+	talloc_free(r1);
+
+	fprintf(stderr, "Checking that p1 is empty and freeing p1\n");
+	CHECK_BLOCKS(__FUNCTION__, p1, 1);
+	talloc_free(p1);
+
+	talloc_report_full(NULL, stderr);
+	CHECK_BLOCKS(__FUNCTION__, root, 1);
+
+	talloc_free(root);
 	printf("success: ref1\n");
 	return true;
 }

+/* check that an allocation that is freed while also referenced finally
goes
+   away when the reference is released */
+static bool test_ref_free(void)
+{
+	void *root, *p1, *p2, *ref, *r1;
+
+	printf("test: test_ref_free\n# FREE ON SINGLE REFERENCE FREE\n");
+	talloc_erase_no_owner_context();
+
+	root = talloc_named_const(NULL, 0, "root");
+	p1 = talloc_named_const(root, 1, "p1");
+	p2 = talloc_named_const(p1, 1, "p2");
+	/* Now root owns p1, and p1 owns p2 */
+	
+	r1 = talloc_named_const(root, 1, "r1");	
+	ref = talloc_reference(r1, p2);
+	/* now r1 has ref reference to p2 */
+	talloc_report_full(root, stderr);
+
+	CHECK_BLOCKS(__FUNCTION__, p1, 2);
+	CHECK_BLOCKS(__FUNCTION__, p2, 1);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);
+
+	fprintf(stderr, "Freeing p2\n");
+	talloc_free(p2);
+	/* r1 should have ref reference to p2 still */
+	talloc_report_full(root, stderr);
+
+	CHECK_BLOCKS(__FUNCTION__, p1, 1);
+	CHECK_BLOCKS(__FUNCTION__, root, 4);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);
+
+	/* if we free r1 then p2 should also vanish */
+	fprintf(stderr, "Freeing r1\n");
+	talloc_free(r1);
+	/* p2 should have gone away  */
+	talloc_report_full(NULL, stderr);
+	CHECK_BLOCKS(__FUNCTION__, root, 2);
+	CHECK_BLOCKS(__FUNCTION__, p1, 1);
+
+	talloc_free(root);
+	printf("success: ref1\n");
+	return true;
+}
+
+/* If an object having references from children that are also referenced is
+   freed, the child reference will be removed, but the child will survive
+   (because of it's reference) and the object will still be freed leaving
+   a dangling reference */
+static bool test_dangling_loop(void)
+{
+	void *root, *p1, *p2, *ref, *r1, *r2;
+
+	printf("test: %s\n# FREE ON SINGLE REFERENCE FREE\n",__FUNCTION__);
+	talloc_erase_no_owner_context();
+
+	root = talloc_named_const(NULL, 0, "root");
+	p1 = talloc_named_const(root, 1, "p1");
+	p2 = talloc_named_const(p1, 1, "p2");
+	/* Now root owns p1, and p1 owns p2 */
+	
+	/* someone takes a ref on p2 */
+	r1 = talloc_named_const(root, 1, "r1");	
+	ref = talloc_reference(r1, p2);
+	
+	/* p2 takes a ref on p1 */
+	talloc_reference(p2, p1);
+	
+	talloc_report_full(NULL, stderr);
+
+	CHECK_BLOCKS(__FUNCTION__, root, 6);
+	CHECK_BLOCKS(__FUNCTION__, p1, 3);
+	CHECK_BLOCKS(__FUNCTION__, p2, 2);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);
+
+	/* talloc will wrongly spot a loop and free p2's ref, and then p1
+	   leaving a dangling pointer */
+	fprintf(stderr, "Freeing p1\n");
+	talloc_free(p1);
+	/* p1 should not get freed as it is referenced from something (a
child) that won't free */
+	/* r1 should have ref reference to p2 still */
+	talloc_report_full(NULL, stderr);
+
+	CHECK_BLOCKS(__FUNCTION__, root, 3);
+	/* The ugly talloc de-child-looping code will delete p2's reference
+	   leaving p2 having a dangling pointer. p2's reference should remain */
+	CHECK_BLOCKS(__FUNCTION__, p2, 2);
+	CHECK_BLOCKS(__FUNCTION__, r1, 2);
+
+	/* if we free r1 then p2 should also vanish as it is owned by
something that
+	   is not owned, but we can't track that yet. Once p2 vanishes it's
reference
+	   to p1 should vanish letting p1 vanish.
+	   We can often make sub-tree's from no-owner-context, by checking
when references
+	   of things child-of no-owner-context die
+	*/
+	fprintf(stderr, "Freeing r1\n");
+	talloc_free(r1);
+	talloc_report_full(NULL, stderr);
+	CHECK_BLOCKS(__FUNCTION__, root, 1);
+
+	talloc_free(root);
+	printf("success: ref1\n");
+	
+	talloc_erase_no_owner_context();
+	talloc_report_full(NULL, stderr);
+	return true;
+}
+
 /*
   measure the speed of talloc versus malloc
 */
@@ -861,6 +1137,7 @@ static bool test_speed(void)
 	struct timeval tv;

 	printf("test: speed\n# TALLOC VS MALLOC SPEED\n");
+	talloc_erase_no_owner_context();

 	tv = timeval_current();
 	count = 0;
@@ -926,18 +1203,29 @@ static bool test_lifeless(void)
 	void *child_owner = talloc_new(NULL);

 	printf("test: lifeless\n# TALLOC_UNLINK LOOP\n");
+	talloc_erase_no_owner_context();

 	parent = talloc_strdup(top, "parent");
 	child = talloc_strdup(parent, "child");
 	(void)talloc_reference(child, parent);
 	(void)talloc_reference(child_owner, child);
-	talloc_report_full(top, stderr);
+	talloc_report_full(NULL, stderr);
 	talloc_unlink(top, parent);
+	/* parent is now owner by no_owner_context */
+	talloc_report_full(NULL, stderr);
 	talloc_free(child);
-	talloc_report_full(top, stderr);
+	/* child also has no owner, but is still referenced */
+	talloc_report_full(NULL, stderr);
+	/* top owns nothing anyway */
 	talloc_free(top);
+	/* child_owner has reference to child which is un-owned */
 	talloc_free(child_owner);
-	talloc_free(child);
+// child will already be free
+//	talloc_free(child);
+
+    /* prove that parent and child have been freed */
+	talloc_report_full(NULL, stderr);
+	CHECK_BLOCKS("lifeless", talloc_no_owner_context (), 1);

 	printf("success: lifeless\n");
 	return true;
@@ -960,6 +1248,7 @@ static bool test_loop(void)
 	} *req1;

 	printf("test: loop\n# TALLOC LOOP DESTRUCTION\n");
+	talloc_erase_no_owner_context();

 	parent = talloc_strdup(top, "parent");
 	req1 = talloc(parent, struct req1);
@@ -994,16 +1283,21 @@ static bool test_free_parent_deny_child(void)
 	char *level3;

 	printf("test: free_parent_deny_child\n# TALLOC FREE PARENT DENY CHILD\n");
+	talloc_erase_no_owner_context();

 	level1 = talloc_strdup(top, "level1");
 	level2 = talloc_strdup(level1, "level2");
 	level3 = talloc_strdup(level2, "level3");

+	fprintf(stderr,"level1 %p\nlevel2 %p\nlevel3 %p\n", level1, level2,
level3);
 	talloc_set_destructor(level3, fail_destructor_str);
 	talloc_free(level1);
 	talloc_set_destructor(level3, NULL);

-	CHECK_PARENT("free_parent_deny_child", level3, top);
+// should it be owned by no_owner_context or by top given that the
destructor held it back?
+// we might want another chance at destructing...
+//	CHECK_PARENT("free_parent_deny_child", level3, top);
+	CHECK_PARENT("free_parent_deny_child", level3, talloc_no_owner_context
());

 	talloc_free(top);

@@ -1024,6 +1318,7 @@ static bool test_talloc_ptrtype(void)
 	const char *location4;

 	printf("test: ptrtype\n# TALLOC PTRTYPE\n");
+	talloc_erase_no_owner_context();

 	s1 = talloc_ptrtype(top, s1);location1 = __location__;

@@ -1110,6 +1405,7 @@ static bool test_talloc_free_in_destructor(void)
 	void **level5;

 	printf("test: free_in_destructor\n# TALLOC FREE IN DESTRUCTOR\n");
+	talloc_erase_no_owner_context();

 	level0 = talloc_new(NULL);
 	level1 = talloc_new(level0);
@@ -1139,6 +1435,7 @@ static bool test_autofree(void)
 	/* autofree test would kill smbtorture */
 	void *p;
 	printf("test: autofree\n# TALLOC AUTOFREE CONTEXT\n");
+	talloc_erase_no_owner_context();

 	p = talloc_autofree_context();
 	talloc_free(p);
@@ -1196,8 +1493,12 @@ bool torture_local_talloc(struct torture_context
*tctx)
 	ret &= test_free_parent_deny_child();
 	ret &= test_talloc_ptrtype();
 	ret &= test_talloc_free_in_destructor();
-	ret &= test_pool();
 	ret &= test_implicit_explicit_free();
+	ret &= test_dangling_loop();
+	ret &= test_ref_free_owner();
+	ret &= test_ref_free_self();
+	ret &= test_ref_free();
+	ret &= test_pool();

 	if (ret) {
 		ret &= test_speed();


More information about the samba-technical mailing list