On Wed, Jan 14, 2009 at 5:05 PM, Regis <[email protected]> wrote:
> Tony Wu wrote:
>>
>> On Tue, Jan 13, 2009 at 4:28 PM, Regis <[email protected]> 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.
it is protected by locks but could be invoked one by one, right? what
about following patch
### Eclipse Workspace Patch 1.0
#P nio
Index: META-INF/MANIFEST.MF
===================================================================
--- META-INF/MANIFEST.MF (revision 720103)
+++ META-INF/MANIFEST.MF (working copy)
@@ -19,6 +19,7 @@
java.nio.charset,
java.security,
java.util,
+ java.util.concurrent.atomic,
org.apache.harmony.kernel.vm,
org.apache.harmony.luni.net,
org.apache.harmony.luni.platform,
Index: src/main/java/common/java/nio/channels/spi/AbstractSelector.java
===================================================================
--- src/main/java/common/java/nio/channels/spi/AbstractSelector.java
(revision
720103)
+++ src/main/java/common/java/nio/channels/spi/AbstractSelector.java (working
copy)
@@ -22,6 +22,7 @@
import java.nio.channels.Selector;
import java.util.HashSet;
import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
/**
* Abstract class for selectors.
@@ -33,7 +34,7 @@
*
*/
public abstract class AbstractSelector extends Selector {
- private volatile boolean isOpen = true;
+ private AtomicBoolean isOpen = new AtomicBoolean(true);
private SelectorProvider provider = null;
@@ -59,8 +60,7 @@
*/
@Override
public synchronized final void close() throws IOException {
- if (isOpen) {
- isOpen = false;
+ if (isOpen.getAndSet(false)) {
implCloseSelector();
}
}
@@ -78,7 +78,7 @@
*/
@Override
public final boolean isOpen() {
- return isOpen;
+ return isOpen.get();
}
/**
>
>>
>>> 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.
>
--
Tony Wu
China Software Development Lab, IBM
