It's desirable that a destroy_notify callback be able to modify the hashtable.  For example, in cairo, we have a legitimate usecase that a destroy_notify removes another item from the hashtable.

Checking the code, I don't think we currently handle that correctly:

static void 
g_hash_table_remove_node (GHashTable   *hash_table, 
                          int           i, 
                          gboolean      notify) 
{ 
  if (notify && hash_table->key_destroy_func) 
    hash_table->key_destroy_func (hash_table->keys[i]); 
 
  if (notify && hash_table->value_destroy_func) 
    hash_table->value_destroy_func (hash_table->values[i]); 
 
  /* Erect tombstone */ 
  hash_table->hashes[i] = 1; 
 
  /* Be GC friendly */ 
  hash_table->keys[i] = NULL; 
  hash_table->values[i] = NULL; 
 
  hash_table->nnodes--; 
} 


In this code, the destroy callbacks may modify the layout of the hashtable (a shrink for example), and hence i would be outdated.

The correct way to handle this is to do all modifications to the table and then call the callback functions.

The case for finish() is less well-defined.  In cairo and harfbuzz, the way we implement it is while (not empty) remove one item.  Another approach would be: while not empty, steal the entire data arrays and then free them.