Skip to content

Commit

Permalink
bpo-40602: _Py_hashtable_set() reports rehash failure (pythonGH-20077)
Browse files Browse the repository at this point in the history
If _Py_hashtable_set() fails to grow the hash table (rehash), it now
fails rather than ignoring the error.
  • Loading branch information
vstinner committed May 14, 2020
1 parent ce21cfc commit d2dc827
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 14 deletions.
14 changes: 9 additions & 5 deletions Modules/_testinternalcapi.c
Expand Up @@ -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++) {
Expand All @@ -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;
Expand All @@ -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);
Expand Down
26 changes: 17 additions & 9 deletions Python/hashtable.c
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);

Expand All @@ -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;
}


Expand Down Expand Up @@ -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);
}


Expand Down

0 comments on commit d2dc827

Please sign in to comment.