[
https://issues.apache.org/jira/browse/ZOOKEEPER-2280?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15943769#comment-15943769
]
ASF GitHub Bot commented on ZOOKEEPER-2280:
-------------------------------------------
Github user eribeiro commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/208#discussion_r108242052
--- Diff: src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java ---
@@ -100,11 +101,15 @@ public void close() {
+ Long.toHexString(sessionId));
}
- synchronized (factory.ipMap) {
- Set<NettyServerCnxn> s =
- factory.ipMap.get(((InetSocketAddress)channel
- .getRemoteAddress()).getAddress());
- s.remove(this);
+ InetAddress address = ((InetSocketAddress)
channel.getRemoteAddress()).getAddress();
+ Set<NettyServerCnxn> s = factory.ipMap.get(address);
+ if (s != null) {
+ synchronized (s) {
--- End diff --
Yeah... in the context of this call I think it is not necessary to
synchronize here if -- as you said -- we use a synchronized set (**currently
it's not**). I would also change `ipMap` to be a `ConcurrentMap` but unsure if
it's worth the effort, tbh. :thinking: thoughts?
> NettyServerCnxnFactory doesn't honor maxClientCnxns param
> ---------------------------------------------------------
>
> Key: ZOOKEEPER-2280
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2280
> Project: ZooKeeper
> Issue Type: Bug
> Components: server
> Affects Versions: 3.4.6, 3.5.0, 3.5.1
> Reporter: Edward Ribeiro
> Assignee: Edward Ribeiro
> Fix For: 3.5.4, 3.6.0
>
> Attachments: ZOOKEEPER-2280.2.patch, ZOOKEEPER-2280.patch
>
>
> Even though NettyServerCnxnFactory has maxClientCnxns (default to 60) it
> doesn't enforce this limit in the code.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
