Code review request 7132889: Possible race conditions in AbstractSelectableChannel

[Shirish Kuncolienkar]

On 8/14/2012 2:01 PM, Alan Bateman wrote:
> On 14/08/2012 03:19, Shirish Kuncolienkar wrote:
>>
>> I see that regLock is not limited to blocking mode. Many places the 
>> keyLock is acquired while the regLock is held.
> It guards the blocking mode, and may have been clearer if it were 
> named blockingLock. Locking regLock and keyLock (in that order) should 
> be fine, just not in implCloseSelectableChannel because you need to be 
> able to invoke close when there is a blocking operation in progress.
>
>> If we do not guard the implCloseSelectableChannel() and key 
>> cancellation within the regLock we would not be able to avoid a key 
>> being added while a channel is closed. Do you think such a behavior 
>> should be documented ?
>> I will run the channel tests as suggested by you.
> Yes, it would be great if you could run the tests with your patch as I 
> am concerned that it will break asynchronous close causing several 
> tests will either hang or deadlock.
>
> I think the residual issue with register returning a valid key can be 
> solved by checking the channel status when holding the keyLock, eg:
>
> synchronized (keyLock) {
>     if (!isOpen())
>         throw new ClosedChannelException();
>     addKey(k);
> }
>
> and change addKey to assert Thread.holdsLock(keyLock);
>
> So I'm curious as to the background to this issue, is this a patch 
> that you have in the IBM JDK?
>
> -Alan.

Alan,

I created a new patch http://cr.openjdk.java.net/~luchsh/webrev.000/ 
based on your suggestions and tested the same against java/nio/channels 
tests

I see no new failures as compared to
java version "1.8.0-ea"
Java(TM) SE Runtime Environment (build 1.8.0-ea-b49)
Java HotSpot(TM) 64-Bit Server VM (build 24.0-b16, mixed mode)

Detailed Test results
passed: 159; failed: 1; error: 1
Error:  java/nio/channels/DatagramChannel/AdaptDatagramSocket.java
FAILED: java/nio/channels/DatagramChannel/MulticastSendReceiveTests.java

The timing window was seen during fixing of the select loop issue and 
the changes are being held as a patch.

Thanks
-Shirish