zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hanm <...@git.apache.org>
Subject [GitHub] zookeeper pull request #211: ZOOKEEPER-2743: Netty connection leaks JMX conn...
Date Thu, 06 Apr 2017 05:35:26 GMT
Github user hanm commented on a diff in the pull request:

    --- Diff: src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java ---
    @@ -87,6 +87,12 @@ public void close() {
                 LOG.debug("close called for sessionid:0x"
                         + Long.toHexString(sessionId));
    +        // ZOOKEEPER-2743:
    +        // Always unregister connection upon close to prevent
    +        // connection bean leak under certain race conditions.
    +        factory.unregisterConnection(this);
    --- End diff --
    That is fine, I might able to provide a formal verification of the theorem but here is
a quick prove of that case:
    * Assume close is called before connection bean is registered [1]
    * The unregister bean in close call is no-op because the bean is not registered. But the
channel will be closed, as part of close call.
    * Now before finalizing session returns, some sort of exception is going to throw, because
the channel is closed. Probably here [2].
    * As part of exception the close is called again. This time it will unregister the bean
(before this fix it will not, so it will miss this edge case.).
    Basically we are safe as close will be called multiple times and guaranteed at least one
close call will happen after cnx bean is registered. 
    [1] https://github.com/apache/zookeeper/blob/master/src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java#L699

If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.

View raw message