bpo-40602: _Py_hashtable_set() reports rehash failure (pythonGH-20077)

vstinner · web-flow · commit d2dc827d1647 · 2020-05-14T22:44:32.000+02:00
If _Py_hashtable_set() fails to grow the hash table (rehash), it now
fails rather than ignoring the error.
diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c
@@ -98,6 +98,11 @@ test_hashtable(PyObject *self, PyObject *Py_UNUSED(args))
         return PyErr_NoMemory();
     }
 
+    // Using an newly allocated table must not crash
+    assert(table->nentries == 0);
+    assert(table->nbuckets > 0);
+    assert(_Py_hashtable_get(table, TO_PTR('x')) == NULL);
+
     // Test _Py_hashtable_set()
     char key;
     for (key='a'; key <= 'z'; key++) {
@@ -121,17 +126,15 @@ test_hashtable(PyObject *self, PyObject *Py_UNUSED(args))
     // Test _Py_hashtable_get()
     for (key='a'; key <= 'z'; key++) {
         void *value_ptr = _Py_hashtable_get(table, TO_PTR(key));
-        int value = (int)FROM_PTR(value_ptr);
-        assert(value == VALUE(key));
+        assert((int)FROM_PTR(value_ptr) == VALUE(key));
     }
 
     // Test _Py_hashtable_steal()
     key = 'p';
     void *value_ptr = _Py_hashtable_steal(table, TO_PTR(key));
-    int value = (int)FROM_PTR(value_ptr);
-    assert(value == VALUE(key));
-
+    assert((int)FROM_PTR(value_ptr) == VALUE(key));
     assert(table->nentries == 25);
+    assert(_Py_hashtable_get_entry(table, TO_PTR(key)) == NULL);
 
     // Test _Py_hashtable_foreach()
     int count = 0;
@@ -142,6 +145,7 @@ test_hashtable(PyObject *self, PyObject *Py_UNUSED(args))
     // Test _Py_hashtable_clear()
     _Py_hashtable_clear(table);
     assert(table->nentries == 0);
+    assert(table->nbuckets > 0);
     assert(_Py_hashtable_get(table, TO_PTR('x')) == NULL);
 
     _Py_hashtable_destroy(table);
diff --git a/Python/hashtable.c b/Python/hashtable.c
@@ -60,7 +60,7 @@
         ((_Py_hashtable_entry_t *)_Py_SLIST_ITEM_NEXT(ENTRY))
 
 /* Forward declaration */
-static void hashtable_rehash(_Py_hashtable_t *ht);
+static int hashtable_rehash(_Py_hashtable_t *ht);
 
 static void
 _Py_slist_init(_Py_slist_t *list)
@@ -198,6 +198,7 @@ _Py_hashtable_steal(_Py_hashtable_t *ht, const void *key)
     ht->alloc.free(entry);
 
     if ((float)ht->nentries / (float)ht->nbuckets < HASHTABLE_LOW) {
+        // Ignore failure: error cannot be reported to the caller
         hashtable_rehash(ht);
     }
     return value;
@@ -228,13 +229,17 @@ _Py_hashtable_set(_Py_hashtable_t *ht, const void *key, void *value)
     entry->key = (void *)key;
     entry->value = value;
 
-    size_t index = entry->key_hash & (ht->nbuckets - 1);
-    _Py_slist_prepend(&ht->buckets[index], (_Py_slist_item_t*)entry);
     ht->nentries++;
-
     if ((float)ht->nentries / (float)ht->nbuckets > HASHTABLE_HIGH) {
-        hashtable_rehash(ht);
+        if (hashtable_rehash(ht) < 0) {
+            ht->nentries--;
+            ht->alloc.free(entry);
+            return -1;
+        }
     }
+
+    size_t index = entry->key_hash & (ht->nbuckets - 1);
+    _Py_slist_prepend(&ht->buckets[index], (_Py_slist_item_t*)entry);
     return 0;
 }
 
@@ -271,19 +276,19 @@ _Py_hashtable_foreach(_Py_hashtable_t *ht,
 }
 
 
-static void
+static int
 hashtable_rehash(_Py_hashtable_t *ht)
 {
     size_t new_size = round_size((size_t)(ht->nentries * HASHTABLE_REHASH_FACTOR));
     if (new_size == ht->nbuckets) {
-        return;
+        return 0;
     }
 
     size_t buckets_size = new_size * sizeof(ht->buckets[0]);
     _Py_slist_t *new_buckets = ht->alloc.malloc(buckets_size);
     if (new_buckets == NULL) {
         /* memory allocation failed */
-        return;
+        return -1;
     }
     memset(new_buckets, 0, buckets_size);
 
@@ -303,6 +308,7 @@ hashtable_rehash(_Py_hashtable_t *ht)
     ht->alloc.free(ht->buckets);
     ht->nbuckets = new_size;
     ht->buckets = new_buckets;
+    return 0;
 }
 
 
@@ -388,7 +394,9 @@ _Py_hashtable_clear(_Py_hashtable_t *ht)
         _Py_slist_init(&ht->buckets[i]);
     }
     ht->nentries = 0;
-    hashtable_rehash(ht);
+    // Ignore failure: clear function is not expected to fail
+    // because of a memory allocation failure.
+    (void)hashtable_rehash(ht);
 }
 
 
