RFR(s): 8205583: Crash in ConcurrentHashTable do_bulk_delete_locked_for

[Gerard Ziemski]

hi Robbin,

Looks good, a few comments:

#1 In stringTable.cpp no need for line:

502   bool interrupted = false;

#2 In concurrentHashTableTasks.inline.hpp, can’t we name the method as “continue”?

 108   void cont(Thread* thread) {

#3 In concurrentHashTableTasks.inline.hpp, I noticed that we grab “_resize_lock” for stuff like bulk delete, so should we rename it to “_task_lock”, or something else that describes its purpose better? (in a new RFR)

 123   BulkDeleteTask(ConcurrentHashTable<VALUE, CONFIG, F>* cht, bool is_mt = false)
 124     : BucketsOperation(cht, is_mt) {
 125   }
 126   // Before start prepare must be called.
 127   bool prepare(Thread* thread) {
 128     bool lock = BucketsOperation::_cht->try_resize_lock(thread);

#4 If we must get “thread_owns_resize_lock” before we call “ setup”, can we enforce it, by calling "thread_owns_resize_lock” from “setup”, or at least should we have assert in “setup” checking that we have the lock?

 132     this->thread_owns_resize_lock(thread);
 133     this->setup();


cheers

> On Jun 25, 2018, at 1:26 PM, Robbin Ehn <robbin.ehn at oracle.com> wrote:
> 
> Hi all, please review.
> 
> Webrev: http://cr.openjdk.java.net/~rehn/8205583/v0/webrev/index.html
> Issue: https://bugs.openjdk.java.net/browse/JDK-8205583
> 
> The problem is that the cancel-able cleaning operation is unaware of rehash.
> The cleaning starts, pauses for a safepoint which does the rehash, destroying
> the old table. When the cleaning continues after the safepoint it will continue
> to do so in the destroyed table.
> 
> The cancelability of the cleaning operation is not needed and just creates
> complicity. In this change-set I remove that functionality, which means a rehash
> will be postponed until the cleaning have finished. (identical as for growing)
> 
> Passed 700 runs of the test that produced the error and tier 1-3.
> 
> Thanks, Robbin