8206471: Race with ConcurrentHashTable deleting items on insert with cleanup thread
Summary: Only fetch Node::next once and use that result.
Reviewed-by: hseigel, dholmes
--- a/src/hotspot/share/utilities/concurrentHashTable.inline.hpp Tue Jul 10 03:14:55 2018 -0400
+++ b/src/hotspot/share/utilities/concurrentHashTable.inline.hpp Tue Jul 10 11:13:33 2018 -0400
@@ -263,8 +263,11 @@
Prefetch::read(*pref->value(), 0);
pref = pref->next();
}
- if (next->next() != NULL) {
- Prefetch::read(*next->next()->value(), 0);
+ // Read next() Node* once. May be racing with a thread moving the next
+ // pointers.
+ Node* next_pref = next->next();
+ if (next_pref != NULL) {
+ Prefetch::read(*next_pref->value(), 0);
}
if (eval_f(next->value())) {
return true;
@@ -546,8 +549,9 @@
lookup_f.equals(rem_n->value(), &is_dead);
if (is_dead) {
ndel[dels++] = rem_n;
- bucket->release_assign_node_ptr(rem_n_prev, rem_n->next());
- rem_n = rem_n->next();
+ Node* next_node = rem_n->next();
+ bucket->release_assign_node_ptr(rem_n_prev, next_node);
+ rem_n = next_node;
if (dels == BULK_DELETE_LIMIT) {
break;
}
@@ -654,32 +658,33 @@
while (aux != NULL) {
bool dead_hash = false;
size_t aux_hash = CONFIG::get_hash(*aux->value(), &dead_hash);
+ Node* aux_next = aux->next();
if (dead_hash) {
delete_me = aux;
// This item is dead, move both list to next
new_table->get_bucket(odd_index)->release_assign_node_ptr(odd,
- aux->next());
+ aux_next);
new_table->get_bucket(even_index)->release_assign_node_ptr(even,
- aux->next());
+ aux_next);
} else {
size_t aux_index = bucket_idx_hash(new_table, aux_hash);
if (aux_index == even_index) {
// This is a even, so move odd to aux/even next
new_table->get_bucket(odd_index)->release_assign_node_ptr(odd,
- aux->next());
+ aux_next);
// Keep in even list
even = aux->next_ptr();
} else if (aux_index == odd_index) {
// This is a odd, so move odd to aux/odd next
new_table->get_bucket(even_index)->release_assign_node_ptr(even,
- aux->next());
+ aux_next);
// Keep in odd list
odd = aux->next_ptr();
} else {
fatal("aux_index does not match even or odd indices");
}
}
- aux = aux->next();
+ aux = aux_next;
// We can only move 1 pointer otherwise a reader might be moved to the wrong
// chain. E.g. looking for even hash value but got moved to the odd bucket
@@ -976,8 +981,9 @@
while (rem_n != NULL) {
if (eval_f(rem_n->value())) {
ndel[dels++] = rem_n;
- bucket->release_assign_node_ptr(rem_n_prev, rem_n->next());
- rem_n = rem_n->next();
+ Node* next_node = rem_n->next();
+ bucket->release_assign_node_ptr(rem_n_prev, next_node);
+ rem_n = next_node;
if (dels == num_del) {
break;
}