diff --git a/CMakeLists.txt b/CMakeLists.txt index 58ee9ed..c6695eb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -267,6 +267,7 @@ if (HAVE_SYS_RANDOM_H) endif() if (HAVE_SYS_RESOURCE_H) check_symbol_exists(getrusage "sys/resource.h" HAVE_GETRUSAGE) + check_symbol_exists(setrlimit "sys/resource.h" HAVE_SETRLIMIT) endif() check_symbol_exists(strtoll "stdlib.h" HAVE_STRTOLL) diff --git a/arraylist.c b/arraylist.c index bfc1425..88c6d23 100644 --- a/arraylist.c +++ b/arraylist.c @@ -172,6 +172,14 @@ int array_list_put_idx(struct array_list *arr, size_t idx, void *data) return 0; } +int array_list_set_idx(struct array_list *arr, size_t idx, void *data) +{ + if (idx >= arr->length) + return -1; + arr->array[idx] = data; + return 0; +} + int array_list_add(struct array_list *arr, void *data) { /* Repeat some of array_list_put_idx() so we can skip several diff --git a/arraylist.h b/arraylist.h index a12f27f..84438ab 100644 --- a/arraylist.h +++ b/arraylist.h @@ -68,6 +68,12 @@ extern int array_list_put_idx(struct array_list *al, size_t i, void *data); 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 size_t array_list_length(struct array_list *al); extern void array_list_sort(struct array_list *arr, int (*compar)(const void *, const void *)); diff --git a/json-c.sym b/json-c.sym index a419013..7a20758 100644 --- a/json-c.sym +++ b/json-c.sym @@ -17,6 +17,7 @@ JSONC_PRIVATE { array_list_free; array_list_new; array_list_put_idx; + array_list_set_idx; array_list_sort; json_hex_chars; json_parse_double; @@ -24,6 +25,7 @@ JSONC_PRIVATE { json_parse_uint64; lh_table_delete; lh_table_delete_entry; + lh_table_delete_entry_to_tail; lh_table_free; lh_table_insert; lh_table_insert_w_hash; 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..b96c74b 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 58e1313..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) { @@ -704,6 +706,22 @@ int lh_table_delete_entry(struct lh_table *t, struct lh_entry *e) return 0; } +int lh_table_delete_entry_to_tail(struct lh_table *t, struct lh_entry *first_entry) +{ + 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; + if (del_entry == first_entry) + break; + del_entry = prev; + } while (del_entry != NULL); + return 0; +} + int lh_table_delete(struct lh_table *t, const void *k) { struct lh_entry *e = lh_table_lookup_entry(t, k); diff --git a/linkhash.h b/linkhash.h index 65c4909..5ad17cf 100644 --- a/linkhash.h +++ b/linkhash.h @@ -303,6 +303,17 @@ extern json_bool lh_table_lookup_ex(struct lh_table *t, const void *k, void **v) */ extern int lh_table_delete_entry(struct lh_table *t, struct lh_entry *e); +/** + * Delete all entries from the specified one to the tail of the list. + * Same as calling lh_table_delete_entry() on each of them. + * + * @param t the table to delete from. + * @param e a pointer to the first entry to delete. + * @return 0 if the item was deleted. + * @return -1 if it was not found. + */ +extern int lh_table_delete_entry_to_tail(struct lh_table *t, struct lh_entry *e); + /** * Delete a record from the table. * diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 46f802d..caff03f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -21,6 +21,7 @@ set(ALL_TEST_NAMES test_charcase test_compare test_deep_copy + test_deep_nesting test_double_serializer test_float test_int_add diff --git a/tests/test_deep_nesting.c b/tests/test_deep_nesting.c new file mode 100644 index 0000000..90fa196 --- /dev/null +++ b/tests/test_deep_nesting.c @@ -0,0 +1,84 @@ +#ifdef NDEBUG +#undef NDEBUG +#endif +#include +#include +#include +#include +#ifdef HAVE_SYS_RESOURCE_H +#include +#endif + +#include "config.h" + +#include "json.h" + +#define NESTING_DEPTH 100000 + +static char *generate_json_string(void); +static char *generate_json_string() +{ + char *str; + int depth = NESTING_DEPTH; + str = malloc(depth * 2 + 1); + memset(str, '[', depth); + memset(str + depth, ']', depth); + str[depth * 2] = '\0'; + return str; +} + +static void test_deep_nesting_put(const char *str); +static void test_deep_nesting_put(const char *str) +{ + json_object *my_array; + + struct json_tokener *tok = json_tokener_new_ex(NESTING_DEPTH); + my_array = json_tokener_parse_ex(tok, str, strlen(str) + 1); + printf("Parsed depth %d string to json_object: %s\n", NESTING_DEPTH, (my_array == NULL) ? "NO" : "yes"); + + json_object_put(my_array); + printf("Freed json_object\n"); + + json_tokener_free(tok); +} + +static void test_deep_nesting_tostring(const char *str); +static void test_deep_nesting_tostring(const char *str) +{ + json_object *my_array; + + struct json_tokener *tok = json_tokener_new_ex(NESTING_DEPTH); + my_array = json_tokener_parse_ex(tok, str, strlen(str) + 1); + printf("Parsed depth %d string to json_object: %s\n", NESTING_DEPTH, (my_array == NULL) ? "NO" : "yes"); + + const char *res = json_object_to_json_string_ext(my_array, JSON_C_TO_STRING_PLAIN); + printf("Serialized to string of length %ld\n", (long)strlen(res)); + json_object_put(my_array); + printf("Freed json_object\n"); + + json_tokener_free(tok); +} + +int main(int argc, char **argv) +{ + char *str; +#ifdef HAVE_SETRLIMIT + struct rlimit rl; + rl.rlim_cur = 2048; + rl.rlim_max = 2048; + setrlimit(RLIMIT_STACK, &rl); +#endif + + str = generate_json_string(); + + MC_SET_DEBUG(1); + + test_deep_nesting_put(str); + + if (0) // TODO: make json_object_to_json_string non-recursive + test_deep_nesting_tostring(str); + + free(str); + + return EXIT_SUCCESS; +} diff --git a/tests/test_deep_nesting.expected b/tests/test_deep_nesting.expected new file mode 100644 index 0000000..0790ee8 --- /dev/null +++ b/tests/test_deep_nesting.expected @@ -0,0 +1,2 @@ +Parsed depth 1000000 string to json_object: yes +Freed json_object diff --git a/tests/test_deep_nesting.test b/tests/test_deep_nesting.test new file mode 120000 index 0000000..58a13f4 --- /dev/null +++ b/tests/test_deep_nesting.test @@ -0,0 +1 @@ +test_basic.test \ No newline at end of file