[jira] Commented: (THRIFT-843) TNonblockingSocket connects without a timeout

["Eric Jensen (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-843?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12901076#action_12901076 ] 

Eric Jensen commented on THRIFT-843:
------------------------------------

thanks.  it looks like it won't work for a few reasons:

1.  initSocket overwrites it with "socket_ = new Socket();"

2.  TNonblockingSocket(SocketChannel socketChannel) which is the only thing that calls  socketChannel.configureBlocking(false) is never called, which means all operations on this so-called non-blocking transport will actually be blocking.  Also, that constructor duplicates the initSocket behaviors which should instead be shared

3.  Since open() used to throw, we can be guaranteed nobody was calling it, so there are almost certainly other source files (at least tests) that need to be modified to support the new api.  We will have to modify our twitter client to do so too.  Also I haven't looked at what "am i open" checking the blocking socket does, but this one seems like it will throw an NPE if you use it before opening, which probably isn't the desired behavior.

4.  It would be nice to offer exactly the same api as the blocking socket, by adding the timeout param to the constructor and having the same default constructor it does.

> TNonblockingSocket connects without a timeout
> ---------------------------------------------
>
>                 Key: THRIFT-843
>                 URL: https://issues.apache.org/jira/browse/THRIFT-843
>             Project: Thrift
>          Issue Type: Bug
>          Components: Java - Library
>            Reporter: Eric Jensen
>            Assignee: Bryan Duxbury
>             Fix For: 0.5
>
>         Attachments: thrift-843.patch
>
>
> Unlike TSocket which takes a timeout in its constructor and uses it as both the connect timeout and the so timeout, TNonblockingSocket only supports a setTimeout method which sets the so timeout but doesn't apply the timeout to the connect operation.  Instead, it calls the convenience method SocketChannel.open in its constructor, which calls a blocking, connection-free connect before we set the socket into non-blocking mode or set an so timeout on it.  
> A solution would be to do something like the following:
> SocketChannel socketChannel = SocketChannel.open();
> this.socket_ = socketChannel.socket();
> socket_.setSoTimeout(timeout_);
> socket_.connect(new InetSocketAddress(host, port), timeout_);
> socketChannel.configureBlocking(false);
> That's a bit weird since in TSocket we do the blocking connect in the open() method instead of the constructor, but it seems like a better fix than to refactor all the non-blocking stuff to also call open().
> Also, the initSocket() method in TNonblockingSocket is invalid entirely (although unreachable) as it would overwrite the Socket from the channel with a new, unconnected blocking one.  

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