svn commit: samba r15824 - in branches/SAMBA_4_0/source/lib/talloc: .

[simo]

Tridge, can you backport this to trunk and SAMBA_3_0 ?

Simo.


On Tue, 2006-05-23 at 03:51 +0000, tridge at samba.org wrote:
> Author: tridge
> Date: 2006-05-23 03:51:44 +0000 (Tue, 23 May 2006)
> New Revision: 15824
> 
> WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=15824
> 
> Log:
> 
> 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
> 
> Modified:
>    branches/SAMBA_4_0/source/lib/talloc/talloc.c
>    branches/SAMBA_4_0/source/lib/talloc/talloc.h
>    branches/SAMBA_4_0/source/lib/talloc/testsuite.c
> 
> 
> Changeset:
> Modified: branches/SAMBA_4_0/source/lib/talloc/talloc.c
> ===================================================================
> --- branches/SAMBA_4_0/source/lib/talloc/talloc.c	2006-05-22 23:48:09 UTC (rev 15823)
> +++ branches/SAMBA_4_0/source/lib/talloc/talloc.c	2006-05-23 03:51:44 UTC (rev 15824)
> @@ -542,7 +542,13 @@
>  	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 @@
>  			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 @@
>  	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;
> +}
> 
> Modified: branches/SAMBA_4_0/source/lib/talloc/talloc.h
> ===================================================================
> --- branches/SAMBA_4_0/source/lib/talloc/talloc.h	2006-05-22 23:48:09 UTC (rev 15823)
> +++ branches/SAMBA_4_0/source/lib/talloc/talloc.h	2006-05-23 03:51:44 UTC (rev 15824)
> @@ -137,6 +137,7 @@
>  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
>  
> 
> Modified: branches/SAMBA_4_0/source/lib/talloc/testsuite.c
> ===================================================================
> --- branches/SAMBA_4_0/source/lib/talloc/testsuite.c	2006-05-22 23:48:09 UTC (rev 15823)
> +++ branches/SAMBA_4_0/source/lib/talloc/testsuite.c	2006-05-23 03:51:44 UTC (rev 15824)
> @@ -824,7 +824,7 @@
>  }
>  
> 
> -BOOL test_lifeless(void)
> +static BOOL test_lifeless(void)
>  {
>  	char *top = talloc_new(NULL);
>  	char *parent, *child; 
> @@ -836,14 +836,57 @@
>  	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;
> +}
> +
> +
>  BOOL torture_local_talloc(struct torture_context *torture) 
>  {
>  	BOOL ret = True;
> @@ -861,6 +904,7 @@
>  	ret &= test_realloc_fn();
>  	ret &= test_type();
>  	ret &= test_lifeless();
> +	ret &= test_loop();
>  	if (ret) {
>  		ret &= test_speed();
>  	}
-- 
Simo Sorce
Samba Team GPL Compliance Officer
email: idra at samba.org
http://samba.org