RFR (S) 8206471: Race with ConcurrentHashTable deleting items on insert with cleanup thread

[David Holmes]

Hi Coleen,

On 7/07/2018 5:41 AM, coleen.phillimore at oracle.com wrote:
> Summary: Only fetch Node::next once and use that result.
> 
> A racing thread could NULL next->next()->next().  The Node itself is 
> stable until the write_synchronize() but the pointers may be updated. 
> See bug for more detail.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/8206471.01/webrev
> bug link https://bugs.openjdk.java.net/browse/JDK-8206471

The change looks good.

Could there be a similar race at:

  552       bucket->release_assign_node_ptr(rem_n_prev, rem_n->next());
  553       rem_n = rem_n->next();

Even if not, it is marginally more performant to only do the 
load-acquire once.

Similarly:

  663       new_table->get_bucket(odd_index)->release_assign_node_ptr(odd,
  664 
aux->next());
  665       new_table->get_bucket(even_index)->release_assign_node_ptr(even,
  666 
aux->next());

combined with:

  685     aux = aux->next();

makes for 3 load-acquire (and 2 if we take the else at line #675).

And again:

  982       bucket->release_assign_node_ptr(rem_n_prev, rem_n->next());
  983       rem_n = rem_n->next();

Thanks,
David
-----

> Tested with SymbolTable changes and tests that failed.  Also tested with 
> mach5 hs-tier1-5 (in progress).
> 
> This is actually Robbin's fix, and my review is that it looks good.
> 
> Thanks,
> Coleen