changeset 51803:0a018efec082

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
author coleenp
date Tue, 10 Jul 2018 11:13:33 -0400
parents 40ef1bb91ee8
children d379f06962cf
files src/hotspot/share/utilities/concurrentHashTable.inline.hpp
diffstat 1 files changed, 17 insertions(+), 11 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/utilities/concurrentHashTable.inline.hpp	Mon Jul 16 16:57:37 2018 +0200
+++ 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;
       }