You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/07/24 09:23:39 UTC
[GitHub] [zookeeper] saltos opened a new pull request #1411: Zookeeper 3112: fd leak due to UnresolvedAddressException on connect.
saltos opened a new pull request #1411:
URL: https://github.com/apache/zookeeper/pull/1411
SocketChannel.connect() can throw different kind of exceptions but ClientCnxnSocketNIO.connect() handles only IOException. This could lead to FD leak when socked is opened but is not connected. We should handle some additional exception classes and close the socket.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1411: Zookeeper 3112: fd leak due to UnresolvedAddressException on connect.
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1411:
URL: https://github.com/apache/zookeeper/pull/1411#issuecomment-663493734
I am sorry.
We won't cut releases for 3.4 release line anymore.
Your fix is good for master and it is very valuable.
Please fix the code for java8.
If you want to run ZK 3.4 then you will have to run a patched version.
Using 3.5 ZK client with 3.4 will work If you do not use new features.
Btw I suggest you to upgrade your servers as well
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] saltos commented on pull request #1411: Zookeeper 3112: fd leak due to UnresolvedAddressException on connect.
Posted by GitBox <gi...@apache.org>.
saltos commented on pull request #1411:
URL: https://github.com/apache/zookeeper/pull/1411#issuecomment-663503324
I already have fix for the master: https://github.com/apache/zookeeper/pull/1410
This one is for 3.4.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] saltos commented on a change in pull request #1411: Zookeeper 3112: fd leak due to UnresolvedAddressException on connect.
Posted by GitBox <gi...@apache.org>.
saltos commented on a change in pull request #1411:
URL: https://github.com/apache/zookeeper/pull/1411#discussion_r459968605
##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNIO.java
##########
@@ -289,6 +289,10 @@ void connect(InetSocketAddress addr) throws IOException {
LOG.error("Unable to open socket to " + addr);
sock.close();
throw e;
+ } catch (RuntimeException e) {
+ LOG.error("Unable to open socket to " + addr);
Review comment:
No, because of Java 6 in this version. Sorry, but I have to use different catch blocks...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] saltos commented on a change in pull request #1411: Zookeeper 3112: fd leak due to UnresolvedAddressException on connect.
Posted by GitBox <gi...@apache.org>.
saltos commented on a change in pull request #1411:
URL: https://github.com/apache/zookeeper/pull/1411#discussion_r459986163
##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNIO.java
##########
@@ -289,6 +289,10 @@ void connect(InetSocketAddress addr) throws IOException {
LOG.error("Unable to open socket to " + addr);
sock.close();
throw e;
+ } catch (RuntimeException e) {
+ LOG.error("Unable to open socket to " + addr);
Review comment:
We still use Zookeeper 3.4.13 because our Zookeeper Server is the same version. We can switch on 3.4.14, but I am not sure about more recent versions like 3.5 or 3.6. I fear that client library of the version 3.5 or 3.6 will not work with Zookeeper Server 3.4...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] saltos commented on a change in pull request #1411: Zookeeper 3112: fd leak due to UnresolvedAddressException on connect.
Posted by GitBox <gi...@apache.org>.
saltos commented on a change in pull request #1411:
URL: https://github.com/apache/zookeeper/pull/1411#discussion_r459984438
##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNIO.java
##########
@@ -289,6 +289,10 @@ void connect(InetSocketAddress addr) throws IOException {
LOG.error("Unable to open socket to " + addr);
sock.close();
throw e;
+ } catch (RuntimeException e) {
+ LOG.error("Unable to open socket to " + addr);
Review comment:
Not this version. This is 3.4.14.
```
<!-- maven properties -->
<maven.compiler.source>1.6</maven.compiler.source>
<maven.compiler.target>1.6</maven.compiler.target>
```
https://github.com/apache/zookeeper/blob/branch-3.4.14/pom.xml#L265
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] saltos closed pull request #1411: Zookeeper 3112: fd leak due to UnresolvedAddressException on connect.
Posted by GitBox <gi...@apache.org>.
saltos closed pull request #1411:
URL: https://github.com/apache/zookeeper/pull/1411
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on a change in pull request #1411: Zookeeper 3112: fd leak due to UnresolvedAddressException on connect.
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1411:
URL: https://github.com/apache/zookeeper/pull/1411#discussion_r459956475
##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNIO.java
##########
@@ -289,6 +289,10 @@ void connect(InetSocketAddress addr) throws IOException {
LOG.error("Unable to open socket to " + addr);
sock.close();
throw e;
+ } catch (RuntimeException e) {
+ LOG.error("Unable to open socket to " + addr);
Review comment:
This code looks like the code above.
Can we use multicatch syntax?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on a change in pull request #1411: Zookeeper 3112: fd leak due to UnresolvedAddressException on connect.
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1411:
URL: https://github.com/apache/zookeeper/pull/1411#discussion_r459971696
##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNIO.java
##########
@@ -289,6 +289,10 @@ void connect(InetSocketAddress addr) throws IOException {
LOG.error("Unable to open socket to " + addr);
sock.close();
throw e;
+ } catch (RuntimeException e) {
+ LOG.error("Unable to open socket to " + addr);
Review comment:
Zookeeper is java8+
As you can see we use lambdas and other language features
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org