Code review request 7132889: Possible race conditions in AbstractSelectableChannel

Shirish Kuncolienkar shirishk at linux.vnet.ibm.com
Tue Aug 7 06:17:37 PDT 2012


On 8/7/2012 3:28 PM, Alan Bateman wrote:
> On 07/08/2012 10:34, Shirish Kuncolienkar wrote:
>> Could I get this change reviewed please
>>
>> code inspection of the AbstractSelectableChannel class shows that a 
>> timing window exists between the channel close and cancellation of 
>> the key. Which can lead to following consequences
>> 1. Selector registered on this channel can select the key while the 
>> channel is closed or is being closed
>> 2. A new key can be added to the channel while the it is closed or is 
>> being closed
>> 3. configureBlocking() can be invoked while the channel is closed or 
>> is being closed
>>
>> One might be able to construct a testcase using a test-specific 
>> subclass of AbstractSelectableChannel, which has a synchronization 
>> point in its implementation of implCloseSelectableChannel() so that 
>> tests can be performed there. However, it would be quite an involved 
>> thing to write with clarity.
>>
>> I have created a patch under http://cr.openjdk.java.net/~luchsh/7132889/
>>
> The main issue in 7132889 has already been fixed in jdk8 and 7u6 via 
> 6346658. Would it be possible to test with a build of one of these 
> releases and tell us if you still run into the original problem?
>
> On the regLock vs. keyLock issue then I would like to review this but 
> I don't have cycles just at the moment as you may be right that the 
> synchronization isn't consistent (the synchronization is covered in 
> various places in the Selector spec so it requires reading that, 
> hopefully I will get time soon but it won't be this week).
>
> -Alan
>
I looked at the fix for 6346658 and that will fix the looping issue as 
reported.  However I feel that there are still issues with regLock vs. 
keyLock as you pointed out that the synchronization is inconsistent.  So 
I  suggest that you look at the change set once you have some free time.
-Shirish



More information about the nio-dev mailing list