diff --git a/arraylist.h b/arraylist.h index 5b79410..84438ab 100644 --- a/arraylist.h +++ b/arraylist.h @@ -72,7 +72,7 @@ extern int array_list_add(struct array_list *al, void *data); * Set the value at index i. Caller is responsible for freeing the previous value. * To automatically free the existing value, use array_list_put_idx() instead. */ -extern int array_list_set_idx(struct array_list *al, size_t i, void *data); +extern int array_list_set_idx(struct array_list *al, size_t i, void *data); extern size_t array_list_length(struct array_list *al); diff --git a/json_object.c b/json_object.c index d67a61a..db79d6e 100644 --- a/json_object.c +++ b/json_object.c @@ -269,11 +269,15 @@ struct json_object *json_object_get(struct json_object *jso) return jso; } -int json_object_put(struct json_object *jso) -{ - if (!jso) - return 0; +/** + * Internal json_object_put function + * Returns 0 if we're done "freeing" the object, either because its memory + * was actually released, or we just needed to decrement the refcount. + * Returns 1 when the object is a non-empty container that still needs to be handled. + */ +static inline int _json_object_put_maybe_free(struct json_object *jso, int free_containers) +{ /* Avoid invalid free and crash explicitly instead of (silently) * segfaulting. */ @@ -287,21 +291,157 @@ int json_object_put(struct json_object *jso) * operating on an already-freed object. */ if (__sync_sub_and_fetch(&jso->_ref_count, 1) > 0) - return 0; #else if (--jso->_ref_count > 0) - return 0; #endif + { + return 0; // All done, caller doesn't need to do anything else + } if (jso->_user_delete) jso->_user_delete(jso, jso->_userdata); + switch (jso->o_type) { - case json_type_object: json_object_object_delete(jso); break; - case json_type_array: json_object_array_delete(jso); break; - case json_type_string: json_object_string_delete(jso); break; - default: json_object_generic_delete(jso); break; + case json_type_object: + if (free_containers || lh_table_length(JC_OBJECT(jso)->c_object) == 0) + { + json_object_object_delete(jso); + break; + } + return 1; + case json_type_array: + // container objects are handled by the caller + if (free_containers || array_list_length(JC_ARRAY(jso)->c_array) == 0) + { + json_object_array_delete(jso); + break; + } + return 1; + case json_type_string: + json_object_string_delete(jso); + break; + default: + json_object_generic_delete(jso); + break; } + return 0; // All done, caller doesn't need to do anything else +} + +int json_object_put(struct json_object *jso) +{ + if (!jso) + return 0; + + if (_json_object_put_maybe_free(jso, 0) == 0) + return 0; + // else, it's a non-empty container object, handle it below + + // Note: jso is now a "zombie" object, _ref_count == 0 but memory not yet released + + /* + * Handle container objects with minimal stack usage. + * Perform depth-first iteration, decrementing ref counts on way down + * and freeing actual memory on the way up. + * Iterate backwards through each container so we can use the tail + * pointer/array length to know where to pick up upon popping up to + * the parent. + */ + + while(jso != NULL) + { + size_t total_slots; + size_t slots_left; + struct lh_entry *cur_entry = NULL; + int retry_main_loop = 0; + + if (jso->o_type == json_type_object) + { + total_slots = lh_table_length(JC_OBJECT(jso)->c_object); + cur_entry = JC_OBJECT(jso)->c_object->tail; + } + else + { + total_slots = array_list_length(JC_ARRAY(jso)->c_array); + } + slots_left = total_slots; + + while (slots_left > 0) + { + size_t cur_slot = slots_left - 1; + json_object *child; + + // First, clear the slot in the current jso object + // The slot itself will be freed when jso is freed, or + // if the child object in the slot is a container too and + // and we "recurse" into it. + switch (jso->o_type) + { + case json_type_object: + child = (json_object *)lh_entry_v(cur_entry); + // We're going to free child, so detach it from the entry + lh_entry_set_val(cur_entry, NULL); + break; + case json_type_array: + child = (struct json_object *)array_list_get_idx(JC_ARRAY(jso)->c_array, cur_slot); + // We're going to free child, so detach it from the entry + array_list_set_idx(JC_ARRAY(jso)->c_array, cur_slot, NULL); + break; + default: + assert(!"jso->o_type is not object or array"); + break; + } + + // Now, handle actually freeing the json_object in that slot + if (!child || _json_object_put_maybe_free(child, 0) == 0) + { + // child is either freed, or still referenced somewhere else + // leave it as-is and handle the previous slot + slots_left--; + if (jso->o_type == json_type_object) + cur_entry = cur_entry->prev; + continue; + } + // _ref_count == 0 now, and _user_delete has been called so we can re-use _userdata + child->_delete_parent = jso; // aka _userdata + child->_user_delete = NULL; // make sure it's not called again + + // Clear the slot entries whose json_object have been freed so when we pop + // back up to this jso we can continue where we left off. + // Note: since we set each entry to NULL above, clearing the slot + // is a noop wrt releasing a json_object. + if (jso->o_type == json_type_object) + { + lh_table_delete_entry_to_tail(JC_OBJECT(jso)->c_object, cur_entry); + } + else // json_type_array + { + array_list_del_idx(JC_ARRAY(jso)->c_array, cur_slot, total_slots - cur_slot); + } + // Iterate down through the child, it will be freed once all + // of *its* children are freed + jso = child; + retry_main_loop = 1; + break; + } + + if (retry_main_loop) + // Iterating down, don't free jso yet + continue; + + // All slots are cleared, now pop back up to the parent + { + json_object *parent = jso->_delete_parent; + // jso is a child that's already been detached from its parent + // so we need to actually free it now + assert(jso->_ref_count == 0); + jso->_ref_count++; // We're the exclusive owner of jso, non-atomic add is ok. + assert(_json_object_put_maybe_free(jso, 1) == 0); + jso = parent; + // iteration will be reset at the top of the loop + } + } + return 1; } @@ -516,9 +656,11 @@ static int json_object_object_to_json_string(struct json_object *jso, struct pri static void json_object_lh_entry_free(struct lh_entry *ent) { + struct json_object *jso = (struct json_object *)lh_entry_v(ent); if (!lh_entry_k_is_constant(ent)) free(lh_entry_k(ent)); - json_object_put((struct json_object *)lh_entry_v(ent)); + if (jso) // micro-opt, skip func call on null object + json_object_put(jso); } static void json_object_object_delete(struct json_object *jso_base) @@ -1497,7 +1639,9 @@ static int json_object_array_to_json_string(struct json_object *jso, struct prin static void json_object_array_entry_free(void *data) { - json_object_put((struct json_object *)data); + struct json_object *jso = (struct json_object *)data; + if (jso) // micro-opt, skip func call on null object + json_object_put(jso); } static void json_object_array_delete(struct json_object *jso) diff --git a/json_object_private.h b/json_object_private.h index f81af88..fe783c7 100644 --- a/json_object_private.h +++ b/json_object_private.h @@ -47,7 +47,10 @@ struct json_object json_object_to_json_string_fn *_to_json_string; struct printbuf *_pb; json_object_delete_fn *_user_delete; - void *_userdata; + union { + void *_userdata; + void *_delete_parent; // Used during json_object_put + }; // Actually longer, always malloc'd as some more-specific type. // The rest of a struct json_object_${o_type} follows }; diff --git a/linkhash.c b/linkhash.c index 1856569..04d1ff5 100644 --- a/linkhash.c +++ b/linkhash.c @@ -668,6 +668,8 @@ int lh_table_delete_entry(struct lh_table *t, struct lh_entry *e) /* CAW: fixed to be 64bit nice, still need the crazy negative case... */ ptrdiff_t n = (ptrdiff_t)(e - t->table); + assert(n >= 0 && n < t->size); + /* CAW: this is bad, really bad, maybe stack goes other direction on this machine... */ if (n < 0) { @@ -709,10 +711,14 @@ int lh_table_delete_entry_to_tail(struct lh_table *t, struct lh_entry *first_ent struct lh_entry *del_entry = t->tail; do { + struct lh_entry *prev = del_entry->prev; // Could probably micro-optimize this, but better to avoid code duplication for now if (lh_table_delete_entry(t, del_entry) != 0) return -1; - } while (del_entry != first_entry); + if (del_entry == first_entry) + break; + del_entry = prev; + } while (del_entry != NULL); return 0; }