harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tony Wu" <wuyue...@gmail.com>
Subject Re: [classlib][nio]Is selector.close() blocking or not?
Date Wed, 14 Jan 2009 08:34:46 GMT
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.

> 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.
>



-- 
Tony Wu
China Software Development Lab, IBM

Mime
View raw message