ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1063

Dubious synchronization in Zookeeper and ClientCnxnSocketNIO classes

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.4.0
    • Component/s: java client
    • Labels:
      None

      Description

      Synchronization around dataWatches, existWatches and childWatches in Zookeeper is incorrect.
      Synchronization around outgoingQueue and pendingQueue in ClientCnxnSocketNIO is incorrect.
      Synchronization around selector and key sets in ClientCnxnSocketNIO seems odd.

      1. ZOOKEEPER-1063.patch
        7 kB
        Yanick Dufresne
      2. ZOOKEEPER-1063.patch
        7 kB
        Yanick Dufresne
      3. ZOOKEEPER-1063.patch
        7 kB
        Mahadev konar

        Activity

        Hide
        Yanick Dufresne added a comment -

        This patch covers the Zookeeper class, and the issues surrounding outgoingQueue and pendingQueue in ClientCnxnSocketNIO. The synchronization around selector also seems wrong to me, but someone with more experience with nio should review this.

        Show
        Yanick Dufresne added a comment - This patch covers the Zookeeper class, and the issues surrounding outgoingQueue and pendingQueue in ClientCnxnSocketNIO. The synchronization around selector also seems wrong to me, but someone with more experience with nio should review this.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12479523/ZOOKEEPER-1063.patch
        against trunk revision 1125179.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/278//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12479523/ZOOKEEPER-1063.patch against trunk revision 1125179. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/278//console This message is automatically generated.
        Hide
        Benjamin Reed added a comment -

        the patch look good, it needs to be freshened.

        Show
        Benjamin Reed added a comment - the patch look good, it needs to be freshened.
        Hide
        Yanick Dufresne added a comment -

        Here's an updated patch, generated against 1127288. Passes all existing tests. No new tests as this patch doesn't introduce any new functionality.

        Show
        Yanick Dufresne added a comment - Here's an updated patch, generated against 1127288. Passes all existing tests. No new tests as this patch doesn't introduce any new functionality.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12480331/ZOOKEEPER-1063.patch
        against trunk revision 1125581.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/284//testReport/
        Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/284//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/284//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12480331/ZOOKEEPER-1063.patch against trunk revision 1125581. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/284//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/284//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-ZOOKEEPER-Build/284//console This message is automatically generated.
        Hide
        Mahadev konar added a comment -

        retrying hudson.

        Show
        Mahadev konar added a comment - retrying hudson.
        Hide
        Mahadev konar added a comment -

        Reuploading Yanick's patch.

        Show
        Mahadev konar added a comment - Reuploading Yanick's patch.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12483901/ZOOKEEPER-1063.patch
        against trunk revision 1139494.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/353//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/353//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/353//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12483901/ZOOKEEPER-1063.patch against trunk revision 1139494. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/353//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/353//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/353//console This message is automatically generated.
        Hide
        Benjamin Reed added a comment -

        Committed revision 1146953.

        Show
        Benjamin Reed added a comment - Committed revision 1146953.
        Hide
        Benjamin Reed added a comment -

        thanx yanick!

        Show
        Benjamin Reed added a comment - thanx yanick!
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #1244 (See https://builds.apache.org/job/ZooKeeper-trunk/1244/)
        ZOOKEEPER-1063. Dubious synchronization in Zookeeper and ClientCnxnSocketNIO classes

        breed : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1146953
        Files :

        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java
        • /zookeeper/trunk/CHANGES.txt
        • /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java
        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #1244 (See https://builds.apache.org/job/ZooKeeper-trunk/1244/ ) ZOOKEEPER-1063 . Dubious synchronization in Zookeeper and ClientCnxnSocketNIO classes breed : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1146953 Files : /zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxnSocketNIO.java /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java

          People

          • Assignee:
            Yanick Dufresne
            Reporter:
            Yanick Dufresne
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development