[jira] Commented: (HARMONY-6663) Time-consuming operation with lock held in ServerSocketChannelImpl.java

["Regis Xu (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HARMONY-6663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12915615#action_12915615 ] 

Regis Xu commented on HARMONY-6663:
-----------------------------------

Hi Wendy,

I think the code intend to hold "acceptLock" when doing "accept", it tries to keep non-blocking accept work correctly. As you said, "acceptLock" may hold for a long time if socket.accept() is blocked, but from other side, there is no new connection is coming, even we remove "synchronized (acceptLock)", Thread B still could not go any further, actually socket.accept is synchronized (check java.net.ServerSocket.implAccept), so I don't think it could cause "significantly downgrade software performance".

I'll close it as "Won't fix", if you have any concern, feel free to re-open it.

> Time-consuming operation with lock held in ServerSocketChannelImpl.java 
> ------------------------------------------------------------------------
>
>                 Key: HARMONY-6663
>                 URL: https://issues.apache.org/jira/browse/HARMONY-6663
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>    Affects Versions: 6.0M1
>         Environment: Windows XP
>            Reporter: Wendy Feng
>   Original Estimate: 7h
>  Remaining Estimate: 7h
>
> I found a time-consuming operation with lock held in modules/nio/src/main/java/common/org/apache/harmony/nio/internal/ServerSocketChannelImpl.java
> public SocketChannel accept() throws IOException {
>       ...
>             synchronized (acceptLock) {
>                     boolean isBlocking = isBlocking();
>                     ...
>                     do {
>                         try {
>                             ((ServerSocketAdapter) socket).accept(socketGot,
>                                     (SocketChannelImpl) sockChannel);
>                             // select successfully, break out immediately.
>                             break;
>                         } catch (SocketTimeoutException e) {
>                             // continue to accept if the channel is in blocking
>                             // mode.
>                         }
>                     } while (isBlocking);
>             }
>         ...
>     }
> Thread A executes accept() method, holds a lock on "acceptLock ", but may block at socket.accept() for a long time.
> Thread B executes accept(), cannot acquire the lock on "acceptLock ".
> Consequence:
> Lock contends, significantly downgrade software performance.
> I suggest to move the do-while block out of the synchronization block as follow:
> public SocketChannel accept() throws IOException {
>       ...
>             boolean isBlocking = isBlocking();
>             synchronized (acceptLock) {   
>                     ...
>              }
>                do {
>                         try {
>                             ((ServerSocketAdapter) socket).accept(socketGot,
>                                     (SocketChannelImpl) sockChannel);
>                             // select successfully, break out immediately.
>                             break;
>                         } catch (SocketTimeoutException e) {
>                             // continue to accept if the channel is in blocking
>                             // mode.
>                         }
>                     } while (isBlocking);
>         ...
>     }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.