harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Regis <xu.re...@gmail.com>
Subject Re: [classlib][nio]Is selector.close() blocking or not?
Date Wed, 14 Jan 2009 09:05:45 GMT
Tony Wu wrote:
> On Tue, Jan 13, 2009 at 4:28 PM, Regis <xu.regis@gmail.com> wrote:
>> Hui Ding wrote:
>>> Hi Experts,
>>> I am writing an application with NIO and Selector and found a strange
>>> issue that if I call "Selector.close()" while a thread is being
>>> blocked on that Selector(By calling "Selector.select()"), the call to
>>> "close()" will block indefinitely until "Selector.select()" returns.
>>> It's said in the Javadoc of selector.close() that: "If a thread is
>>> currently blocked in one of this selector's selection methods then it
>>> is interrupted as if by invoking the selector's wakeup method". So I
>>> guess under no circumstances should Select.close() block, right?
>>> However I came across another topic about the concurrency of Selector:
>>> "The selection operations synchronize on the selector itself, on the
>>> key set, and on the selected-key set, in that order.
>>> The close method synchronizes on the selector and all three key sets
>>> in the same order as in a selection operation." If it is true that
>>> Selector.select() and Selector.close() both synchronize on the
>>> selector itself, I think Selector.close() will block if other threads
>>> are blocked on Selector.select().
>>>
>>> Now I am confused about which one is the correct behavior of
>>> Selector.close(), blocking or not?
>>>
>>> BTW. I gave it a try on a Desktop PC with SUN JDK and Harmony JDK, seems
>>> with SUN JDK, Selector.close() will never block while with Harmony
>>> JDK, if another thread is being blocked on Selector.select(),
>>> Selector.close will block until select() returns. Is this a bug with
>>> Harmony JDK or not?
>>>
>> Hi,
>>
>> Could you help to try the following patch?
>>
>> Index:
>> modules/nio/src/main/java/common/java/nio/channels/spi/AbstractSelector.java
>> =====================================================================
>> ---
>> modules/nio/src/main/java/common/java/nio/channels/spi/AbstractSelector.java
>> +++
>> modules/nio/src/main/java/common/java/nio/channels/spi/AbstractSelector.java
>> @@ -58,7 +58,7 @@ public abstract class AbstractSelector extends Selector {
>>      * @see java.nio.channels.Selector#close()
>>      */
>>     @Override
>> -    public synchronized final void close() throws IOException {
>> +    public final void close() throws IOException {
>>         if (isOpen) {
>>             isOpen = false;
>>             implCloseSelector();
> 
> Hi Regis,
> 
> I've not run with this patch but I think there is a pontential risk of
> running implCloseSelector() more than once. with patch above, several
> threads might enter the "if" block in the same time if they all want
> to close the Selector. and then the wakeup() and deregister() will be
> called many times.
Yes, but for wakeup(), according to the spec:
"Invoking this method more than once between two successive selection 
operations has the same effect as invoking it just once."
so I think it's OK to invoke it many times.

And for deregister(), it should be invoke only once. Because it is 
protected by these locks, once the first thread enter the block and 
execute successfully, the "keys" should be empty, so deregister() won't 
be called any more. doCancel() is the similar case.

> 
>> Index:
>> modules/nio/src/main/java/common/org/apache/harmony/nio/internal/SelectorImpl.java
>> =====================================================================
>> ---
>> modules/nio/src/main/java/common/org/apache/harmony/nio/internal/SelectorImpl.java
>> +++
>> modules/nio/src/main/java/common/org/apache/harmony/nio/internal/SelectorImpl.java
>> @@ -138,6 +138,7 @@ final class SelectorImpl extends AbstractSelector {
>>      */
>>     @Override
>>     protected void implCloseSelector() throws IOException {
>> +        wakeup();
>>         synchronized (this) {
>>             synchronized (keysSet) {
>>                 synchronized (selectedKeys) {
>> @@ -147,7 +148,6 @@ final class SelectorImpl extends AbstractSelector {
>>                             deregister((AbstractSelectionKey) sk);
>>                         }
>>                     }
>> -                    wakeup();
>>                 }
>>             }
>>         }
>>
>>
>> after the patch, Harmony could pass the following test as RI:
>>
>>        Thread thread = new Thread(new Runnable() {
>>            public void run() {
>>                int i = 0;
>>                try {
>>                    if (selector.select() != 0) {
>>                        throw new RuntimeException("invalid return value: " +
>> i);
>>                    }
>>                } catch (IOException e) {
>>                    // TODO Auto-generated catch block
>>                    e.printStackTrace();
>>                }
>>            }
>>        });
>>        thread.start();
>>        Thread.sleep(1000);
>>
>>        selector.close();
>>        System.out.println("selector is closed");
>>
>> --
>> Best Regards,
>> Regis.
>>
> 
> 
> 


-- 
Best Regards,
Regis.

Mime
View raw message