# HG changeset patch # User coleenp # Date 1531235613 14400 # Node ID e0028bb6dd3df751bede76da156ebfcb0f487c60 # Parent 3f51ddbe48434e6f9ba2ee3233e21279769839ed 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 diff -r 3f51ddbe4843 -r e0028bb6dd3d src/hotspot/share/utilities/concurrentHashTable.inline.hpp --- 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; }