From: Stefan Metzmacher Date: Mon, 13 Jul 2009 16:49:54 +0000 (+0200) Subject: talloc: make the no_owner feature simpler and working X-Git-Url: http://git.samba.org/?a=commitdiff_plain;ds=sidebyside;h=c4e789209437e2e5c75c2743fb8ebacee63a80df;p=metze%2Fsamba%2Fwip.git talloc: make the no_owner feature simpler and working metze --- diff --git a/lib/talloc/talloc.c b/lib/talloc/talloc.c index 140da5304767..cfa1cc99a7ae 100644 --- a/lib/talloc/talloc.c +++ b/lib/talloc/talloc.c @@ -63,6 +63,7 @@ #define TALLOC_FLAG_POOL 0x04 /* This is a talloc pool */ #define TALLOC_FLAG_POOLMEM 0x08 /* This is allocated in a pool */ #define TALLOC_MAGIC_REFERENCE ((const char *)1) +#define TALLOC_MAGIC_NO_OWNER ((void *)1) /* by default we abort when given a bad pointer (such as when talloc_free() is called on a pointer that came from malloc() */ @@ -103,8 +104,7 @@ */ static void *null_context; static void *autofree_context; -static void *no_owner_context; -void *talloc_no_owner_context(void); +static void *no_owner_context = TALLOC_MAGIC_NO_OWNER; static inline int _talloc_free(void *ptr); struct talloc_reference_handle { @@ -162,6 +162,11 @@ static void talloc_abort_double_free(void) talloc_abort("Bad talloc magic value - double free"); } +static void talloc_abort_no_owner_free(void) +{ + talloc_abort("Bad talloc parent - no_owner free"); +} + static void talloc_abort_unknown_value(void) { talloc_abort("Bad talloc magic value - unknown value"); @@ -209,6 +214,23 @@ do { \ if ((p) && ((p) != (list))) (p)->next = (p)->prev = NULL; \ } while (0) +static inline talloc_parent_is_no_owner(const void *ptr) +{ + struct talloc_chunk *tc; + struct talloc_chunk *no_owner_tc = TALLOC_MAGIC_NO_OWNER; + + tc = talloc_chunk_from_ptr(ptr); + + if (no_owner_context != TALLOC_MAGIC_NO_OWNER) { + no_owner_tc = talloc_chunk_from_ptr(no_owner_context); + } + + if (tc->parent == no_owner_tc) { + return true; + } + + return false; +} /* return the parent chunk of a pointer @@ -224,6 +246,10 @@ static inline struct talloc_chunk *talloc_parent_chunk(const void *ptr) tc = talloc_chunk_from_ptr(ptr); while (tc->prev) tc=tc->prev; + if (unlikely(tc->parent == TALLOC_MAGIC_NO_OWNER)) { + return NULL; + } + return tc->parent; } @@ -429,10 +455,17 @@ 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 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); + talloc_parent_is_no_owner(handle->ptr)) { + int ret = _talloc_free(handle->ptr); + if (ret != 0) { + _TLIST_ADD(ptr_tc->refs, handle); + return ret; + } } return 0; } @@ -494,6 +527,41 @@ void *_talloc_reference(const void *context, const void *ptr) return handle->ptr; } +static inline void _talloc_free_children(struct talloc_chunk *tc) +{ + void *ptr = TC_PTR_FROM_CHUNK(tc); + + tc->flags |= TALLOC_FLAG_LOOP; + + while (tc->child) { + void *child = TC_PTR_FROM_CHUNK(tc->child); + if (unlikely(_talloc_free(child) == -1)) { + const void *new_parent = no_owner_context; + struct talloc_chunk *p = talloc_parent_chunk(ptr); + + /* + * we need to work out who will own an abandoned child + * if it cannot be freed. In priority order, the first + * choice is our parent, and the final choice is the + * no owner context. + */ + 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 (!talloc_parent_is_no_owner(child)) { + talloc_steal(new_parent, child); + } + } + } + + tc->flags &= ~TALLOC_FLAG_LOOP; +} /* internal talloc_free call @@ -526,8 +594,11 @@ static inline int _talloc_free(void *ptr) _talloc_free(tc->refs); return _talloc_free(ptr); } else { - /* we can't free if we have refs, so no_owner_context becomes the owner */ - _talloc_steal(talloc_no_owner_context(), ptr); + /* + * we can't free if we have refs, + * so no_owner_context becomes the owner + */ + _talloc_steal(no_owner_context, ptr); } return -1; } @@ -560,17 +631,7 @@ static inline int _talloc_free(void *ptr) if (tc->next) tc->next->prev = tc->prev; } - tc->flags |= TALLOC_FLAG_LOOP; - - while (tc->child) { - void *child = TC_PTR_FROM_CHUNK(tc->child); - const void *new_parent = talloc_no_owner_context(); - /* 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)) { - talloc_steal(new_parent, child); - } - } + _talloc_free_children(tc); tc->flags |= TALLOC_FLAG_FREE; @@ -612,6 +673,15 @@ void *_talloc_steal(const void *new_ctx, const void *ptr) return NULL; } + if (unlikely(talloc_parent_is_no_owner(ptr) && + new_ctx != no_owner_context)) { + talloc_abort_no_owner_free(); + } + + if (unlikely(new_ctx == TALLOC_MAGIC_NO_OWNER)) { + new_ctx = NULL; + } + if (unlikely(new_ctx == NULL)) { new_ctx = null_context; } @@ -724,7 +794,7 @@ int talloc_unlink(const void *context, void *ptr) return _talloc_free(ptr); } - talloc_steal(talloc_no_owner_context(), ptr); + talloc_steal(no_owner_context, ptr); return 0; } @@ -896,26 +966,7 @@ void talloc_free_children(void *ptr) tc = talloc_chunk_from_ptr(ptr); - while (tc->child) { - /* we need to work out who will own an abandoned child - if it cannot be freed. In priority order, the first - choice is owner of any remaining reference to this - 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 = talloc_no_owner_context(); - - if (unlikely(tc->child->refs)) { - new_parent = talloc_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); - } - talloc_steal(new_parent, child); - } - } + _talloc_free_children(tc); if ((tc->flags & TALLOC_FLAG_POOL) && (*talloc_pool_objectcount(tc) == 1)) { @@ -971,8 +1022,13 @@ int talloc_free(void *ptr) tc = talloc_chunk_from_ptr(ptr); - if (no_owner_context && tc->parent == no_owner_context) { - talloc_abort_double_free(); + if (unlikely(tc->flags & TALLOC_FLAG_LOOP)) { + /* we have a free loop - stop looping */ + return 0; + } + + if (unlikely(talloc_parent_is_no_owner(ptr))) { + talloc_abort_no_owner_free(); } return _talloc_free(ptr); @@ -1224,7 +1280,7 @@ static void talloc_report_depth_FILE_helper(const void *ptr, int depth, int max_ FILE *f = (FILE *)_f; if (is_ref) { - if (talloc_parent(ptr) == no_owner_context && no_owner_context) { + if (talloc_parent_is_no_owner(ptr)) { fprintf(f, "%*sreference to: %s [no owner]\n", depth*4, "", name); } else { fprintf(f, "%*sreference to: %s\n", depth*4, "", name); @@ -1317,6 +1373,8 @@ void talloc_enable_null_tracking(void) { if (null_context == NULL) { null_context = _talloc_named_const(NULL, 0, "null_context"); + no_owner_context = _talloc_named_const(null_context, 0, + "no_owner_context"); } } @@ -1327,6 +1385,7 @@ void talloc_disable_null_tracking(void) { _talloc_free(null_context); null_context = NULL; + no_owner_context = TALLOC_MAGIC_NO_OWNER; } /* @@ -1724,29 +1783,6 @@ 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;