[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13115925#comment-13115925
 ] 

[email protected] commented on ZOOKEEPER-786:
---------------------------------------------------------



bq.  On 2011-09-27 13:59:22, Camille Fournier wrote:
bq.  > src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java, line 195
bq.  > <https://reviews.apache.org/r/1714/diff/1/?file=37950#file37950line195>
bq.  >
bq.  >     Why was this removed? It is not germane to the patch at hand.
bq.  
bq.  Thomas Koch wrote:
bq.      You're right, the removal had nothing to do with the issue, but it was 
the right thing to do, once I've been there. It may happen in rare cases, that 
connect returns true already there, but primeConnection() is called anyways 
later from doTransport(). Having to possible call origins only adds to 
uncertainty.
bq.      Did you observe any problems with this?
bq.  
bq.  Camille Fournier wrote:
bq.      Are you sure that the lines in doTransport() will be called and 
executed if the socket connects immediately? I don't personally know whether 
that is the case or not. You didn't write any tests to show that this would 
happen, and you shouldn't have made the change for purposes of removing 
uncertainty without explicitly calling out what you were doing. If you are 
quite sure that even in the case of an immediate connection, OP_CONNECT will be 
set and we will hit that in doTransport, I'm fine with this change. But I don't 
like having changes like this snuck in under the radar, because if this is not 
the case and it causes a bug, it will be a massive pain to find.
bq.  
bq.  Camille Fournier wrote:
bq.      It seems to me, by the by, that if a socket is immediately connected 
it will NOT set the selection key for OP_CONNECT, because state will be set to 
ST_CONNECTED (see the source for SocketChannelImpl, specifically connect:
bq.      
bq.      
http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/sun/nio/ch/SocketChannelImpl.java#SocketChannelImpl.connect%28java.net.SocketAddress%29
bq.      
bq.      and translateReadyOps:
bq.      
bq.      
http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/sun/nio/ch/SocketChannelImpl.java#SocketChannelImpl.translateReadyOps%28int%2Cint%2Csun.nio.ch.SelectionKeyImpl%29
bq.      
bq.      So, I'm not sure that this is a safe change to make, and you should 
always, always, always, be careful when changing anything to do with our NIO 
code unless you are really sure of the consequences.
bq.  
bq.  Patrick Hunt wrote:
bq.      Camille, you're right, I missed this change. Thanks.
bq.      
bq.      There is definitely a reason for this! The connect call can return 
immediately with a connected indication, in which case we need to run the 
primeConnection code, if you don't OP_CONNECT will never be indicated (because 
the connection already took place) and primeConnection will therefore never be 
called in doTransport.
bq.      
bq.      I believe we saw this with Solaris (there was a bug on this but I 
can't find it either on apache nor sourceforge).
bq.      
bq.
bq.  
bq.  Camille Fournier wrote:
bq.      Ted deserves the credit, he pointed it out as part of 1174. If you 
don't mind, I will pull this bit out of the change in trunk when I check in 
1174 there.

It would be better to keep the changes distinct. Would you mind committing this 
snippet first, as a second patch for 786, then apply 1174? Or I/you can 
entirely roll back this change from trunk, you can commit 1174, and then Thomas 
can update this patch and reapply. Which would you prefer?


- Patrick


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1714/#review2090
-----------------------------------------------------------


On 2011-09-14 22:18:24, Thomas Koch wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/1714/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-09-14 22:18:24)
bq.  
bq.  
bq.  Review request for zookeeper.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  .
bq.  
bq.  
bq.  This addresses bug ZOOKEEPER-786.
bq.      https://issues.apache.org/jira/browse/ZOOKEEPER-786
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java 626da04 
bq.  
bq.  Diff: https://reviews.apache.org/r/1714/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Thomas
bq.  
bq.


                
> Exception in ZooKeeper.toString
> -------------------------------
>
>                 Key: ZOOKEEPER-786
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-786
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: java client
>    Affects Versions: 3.3.1
>         Environment: Mac OS X, x86
>            Reporter: Stephen Green
>            Assignee: Thomas Koch
>            Priority: Minor
>             Fix For: 3.5.0
>
>         Attachments: ZOOKEEPER-786.patch
>
>
> When trying to call ZooKeeper.toString during client disconnections, an 
> exception can be generated:
> [04/06/10 15:39:57.744] ERROR Error while calling watcher 
> java.lang.Error: java.net.SocketException: Socket operation on non-socket
>       at sun.nio.ch.Net.localAddress(Net.java:128)
>       at sun.nio.ch.SocketChannelImpl.localAddress(SocketChannelImpl.java:430)
>       at sun.nio.ch.SocketAdaptor.getLocalAddress(SocketAdaptor.java:147)
>       at java.net.Socket.getLocalSocketAddress(Socket.java:717)
>       at 
> org.apache.zookeeper.ClientCnxn.getLocalSocketAddress(ClientCnxn.java:227)
>       at org.apache.zookeeper.ClientCnxn.toString(ClientCnxn.java:183)
>       at java.lang.String.valueOf(String.java:2826)
>       at java.lang.StringBuilder.append(StringBuilder.java:115)
>       at org.apache.zookeeper.ZooKeeper.toString(ZooKeeper.java:1486)
>       at java.util.Formatter$FormatSpecifier.printString(Formatter.java:2794)
>       at java.util.Formatter$FormatSpecifier.print(Formatter.java:2677)
>       at java.util.Formatter.format(Formatter.java:2433)
>       at java.util.Formatter.format(Formatter.java:2367)
>       at java.lang.String.format(String.java:2769)
>       at com.echonest.cluster.ZooContainer.process(ZooContainer.java:544)
>       at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:488)
> Caused by: java.net.SocketException: Socket operation on non-socket
>       at sun.nio.ch.Net.localInetAddress(Native Method)
>       at sun.nio.ch.Net.localAddress(Net.java:125)
>       ... 15 more

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to