RFR[11] of JDK-8208280,java/nio/channels/Selector/RegisterDuringSelect.java fails with "key not removed from key set"

[Alan Bateman]

On 10/08/2018 06:14, Hamlin Li wrote:
>
>
> On 2018/8/9 6:51 PM, Alan Bateman wrote:
>>
>> No objection to dropping the use of phaser from this test but I'm 
>> concerned that this version isn't providing any guarantee that it's 
>> testing that these methods can be called while a selection operation 
>> is in progress. It's similar to the discussion we had here about the 
>> SelectAndClose test in (JDK-8203765) where we converged on spinning 
>> until the the selectedKey set was locked as an indication that the 
>> selection operation is in progress. I think we can do something 
>> similar here. I'm happy to take this one if you'd like.
> Sure, it's updated, please help to review it: 
> http://cr.openjdk.java.net/~mli/8208280/webrev.02/
This looks much better.

It would be good to put javadoc style method description on 
mightHoldLock and spinUntilLock as they will be used by several tests. 
Also I assume you have a typo in the method name and it should be 
"spinUntilLocked".

The update to the SelectAndClose test looks okay.

The re-implementation of the RegisterDuringSelector test is okay but I 
think needs a bit of clean-up to make it easier to maintain. One 
suggestion is to add this function:

     interface TestOperation {
         void accept(Thread t, Selector sel) throws IOException;
     }

Then rename Select to Test have it define a test(TestOperation op) 
method. That will allow to use a lambda expression and needing to 
override readTest in each of the tests. Also having the Thread and 
Selector provided as parameters makes it easier to see where they are 
coming from.

Another thing is that sel and the other fields should be final.

-Alan

-Alan