diff --git a/glib/ghash.c b/glib/ghash.c index 4204f51a4..c7b7139a3 100644 --- a/glib/ghash.c +++ b/glib/ghash.c @@ -557,16 +557,59 @@ g_hash_table_remove_node (GHashTable *hash_table, } +/* + * g_hash_table_setup_storage: + * @hash_table: our #GHashTable + * + * Initialise the hash table size, mask, mod, and arrays. + */ +static void +g_hash_table_setup_storage (GHashTable *hash_table) +{ + gboolean small; + + /* We want to use small arrays only if: + * - we are running on a system where that makes sense (64 bit); and + * - we are not running under valgrind. + */ + small = FALSE; + +#ifdef USE_SMALL_ARRAYS + small = TRUE; + +# ifdef ENABLE_VALGRIND + if (RUNNING_ON_VALGRIND) + small = FALSE; +# endif +#endif + + g_hash_table_set_shift (hash_table, HASH_TABLE_MIN_SHIFT); + + hash_table->have_big_keys = !small; + hash_table->have_big_values = !small; + + hash_table->keys = g_hash_table_realloc_key_or_value_array (NULL, hash_table->size, hash_table->have_big_keys); + hash_table->values = hash_table->keys; + hash_table->hashes = g_new0 (guint, hash_table->size); +} + /* * g_hash_table_remove_all_nodes: * @hash_table: our #GHashTable * @notify: %TRUE if the destroy notify handlers are to be called * - * Removes all nodes from the table. Since this may be a precursor to - * freeing the table entirely, no resize is performed. + * Removes all nodes from the table. * * If @notify is %TRUE then the destroy notify functions are called * for the key and value of the hash node. + * + * Since this may be a precursor to freeing the table entirely, we'd + * ideally perform no resize, and we can indeed avoid that in some + * cases. However: in the case that we'll be making callbacks to user + * code (via destroy notifies) we need to consider that the user code + * might call back into the table again. In this case, we setup a new + * set of arrays so that any callers will see an empty (but valid) + * table. */ static void g_hash_table_remove_all_nodes (GHashTable *hash_table, @@ -580,6 +623,8 @@ g_hash_table_remove_all_nodes (GHashTable *hash_table, gpointer *old_keys; gpointer *old_values; guint *old_hashes; + gboolean old_have_big_keys; + gboolean old_have_big_values; /* If the hash table is already empty, there is nothing to be done. */ if (hash_table->nnodes == 0) @@ -588,6 +633,7 @@ g_hash_table_remove_all_nodes (GHashTable *hash_table, hash_table->nnodes = 0; hash_table->noccupied = 0; + /* Easy case: no callbacks, so we just zero out the arrays */ if (!notify || (hash_table->key_destroy_func == NULL && hash_table->value_destroy_func == NULL)) @@ -608,43 +654,52 @@ g_hash_table_remove_all_nodes (GHashTable *hash_table, return; } - /* Keep the old storage space around to iterate over it. */ + /* Hard case: we need to do user callbacks. There are two + * possibilities here: + * + * 1) there are no outstanding references on the table and therefore + * nobody should be calling into it again (destroying == true) + * + * 2) there are outstanding references, and there may be future + * calls into the table, either after we return, or from the destroy + * notifies that we're about to do (destroying == false) + * + * We handle both cases by taking the current state of the table into + * local variables and replacing it with something else: in the "no + * outstanding references" cases we replace it with a bunch of + * null/zero values so that any access to the table will fail. In the + * "may receive future calls" case, we reinitialise the struct to + * appear like a newly-created empty table. + * + * In both cases, we take over the references for the current state, + * freeing them below. + */ old_size = hash_table->size; - old_keys = hash_table->keys; - old_values = hash_table->values; - old_hashes = hash_table->hashes; + old_have_big_keys = hash_table->have_big_keys; + old_have_big_values = hash_table->have_big_values; + old_keys = g_steal_pointer (&hash_table->keys); + old_values = g_steal_pointer (&hash_table->values); + old_hashes = g_steal_pointer (&hash_table->hashes); - /* Now create a new storage space; If the table is destroyed we can use the - * shortcut of not creating a new storage. This saves the allocation at the - * cost of not allowing any recursive access. - * However, the application doesn't own any reference anymore, so access - * is not allowed. If accesses are done, then either an assert or crash - * *will* happen. */ - g_hash_table_set_shift (hash_table, HASH_TABLE_MIN_SHIFT); if (!destruction) - { - hash_table->keys = g_hash_table_realloc_key_or_value_array (NULL, hash_table->size, FALSE); - hash_table->values = hash_table->keys; - hash_table->hashes = g_new0 (guint, hash_table->size); - } + /* Any accesses will see an empty table */ + g_hash_table_setup_storage (hash_table); else - { - hash_table->keys = NULL; - hash_table->values = NULL; - hash_table->hashes = NULL; - } + /* Will cause a quick crash on any attempted access */ + hash_table->size = hash_table->mod = hash_table->mask = 0; + /* Now do the actual destroy notifies */ for (i = 0; i < old_size; i++) { if (HASH_IS_REAL (old_hashes[i])) { - key = g_hash_table_fetch_key_or_value (old_keys, i, hash_table->have_big_keys); - value = g_hash_table_fetch_key_or_value (old_values, i, hash_table->have_big_values); + key = g_hash_table_fetch_key_or_value (old_keys, i, old_have_big_keys); + value = g_hash_table_fetch_key_or_value (old_values, i, old_have_big_values); old_hashes[i] = UNUSED_HASH_VALUE; - g_hash_table_assign_key_or_value (old_keys, i, hash_table->have_big_keys, NULL); - g_hash_table_assign_key_or_value (old_values, i, hash_table->have_big_values, NULL); + g_hash_table_assign_key_or_value (old_keys, i, old_have_big_keys, NULL); + g_hash_table_assign_key_or_value (old_values, i, old_have_big_values, NULL); if (hash_table->key_destroy_func != NULL) hash_table->key_destroy_func (key); @@ -654,9 +709,6 @@ g_hash_table_remove_all_nodes (GHashTable *hash_table, } } - hash_table->have_big_keys = FALSE; - hash_table->have_big_values = FALSE; - /* Destroy old storage space. */ if (old_keys != old_values) g_free (old_values); @@ -1017,10 +1069,8 @@ g_hash_table_new_full (GHashFunc hash_func, GDestroyNotify value_destroy_func) { GHashTable *hash_table; - gboolean small; hash_table = g_slice_new (GHashTable); - g_hash_table_set_shift (hash_table, HASH_TABLE_MIN_SHIFT); g_atomic_ref_count_init (&hash_table->ref_count); hash_table->nnodes = 0; hash_table->noccupied = 0; @@ -1031,27 +1081,8 @@ g_hash_table_new_full (GHashFunc hash_func, #endif hash_table->key_destroy_func = key_destroy_func; hash_table->value_destroy_func = value_destroy_func; - hash_table->keys = g_hash_table_realloc_key_or_value_array (NULL, hash_table->size, FALSE); - hash_table->values = hash_table->keys; - hash_table->hashes = g_new0 (guint, hash_table->size); - /* We want to use small arrays only if: - * - we are running on a system where that makes sense (64 bit); and - * - we are not running under valgrind. - */ - small = FALSE; - -#ifdef USE_SMALL_ARRAYS - small = TRUE; - -# ifdef ENABLE_VALGRIND - if (RUNNING_ON_VALGRIND) - small = FALSE; -# endif -#endif - - hash_table->have_big_keys = !small; - hash_table->have_big_values = !small; + g_hash_table_setup_storage (hash_table); return hash_table; }