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

Hudson commented on ZOOKEEPER-1972:
-----------------------------------

SUCCESS: Integrated in ZooKeeper-trunk #2379 (See 
[https://builds.apache.org/job/ZooKeeper-trunk/2379/])
ZOOKEEPER-1972. Fix invalid volatile long/int increment (++) (Hongchao Deng via 
phunt) (phunt: 
http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1612015)
* /zookeeper/trunk/CHANGES.txt
* 
/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java
* 
/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java
* /zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/Leader.java
* 
/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java
* 
/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java


> Fix invalid volatile long/int increment (++)
> --------------------------------------------
>
>                 Key: ZOOKEEPER-1972
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1972
>             Project: ZooKeeper
>          Issue Type: Sub-task
>    Affects Versions: 3.5.0
>            Reporter: Hongchao Deng
>            Assignee: Hongchao Deng
>             Fix For: 3.5.0
>
>         Attachments: ZOOKEEPER-1972.patch, ZOOKEEPER-1972.patch.2, 
> ZOOKEEPER-1972.patch.3, ZOOKEEPER-1972.patch.4
>
>
> The findbugs complains about incrementing a volatile variable in 
> [AuthFastLeaderElection|https://github.com/apache/zookeeper/blob/087abf83684167ae56864fe4c3be0079fa653266/src/java/main/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java#L737]
>  and 
> [FastLeaderElection|https://github.com/apache/zookeeper/blob/087abf83684167ae56864fe4c3be0079fa653266/src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java]:
> {code}
> volatile long logicalclock; /* Election instance */
> ...
> logicalclock++;
> {code}
> Actually this is a bug. It should use AtomicLong here instead of volatile.
> Leader.java and 
> [QuorumPeer.java|https://github.com/apache/zookeeper/blob/087abf83684167ae56864fe4c3be0079fa653266/src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java#L428]
>  and LearnerHandler.java:
> {code}
> volatile int tick;
> {code}
> I don't think it needs volatile here. The tick is incremented only in 
> [Leader.java|https://github.com/apache/zookeeper/blob/087abf83684167ae56864fe4c3be0079fa653266/src/java/main/org/apache/zookeeper/server/quorum/Leader.java#L590]:
> {code}
>   synchronized (this) {
>           ...
>                     if (!tickSkip) {
>                         self.tick++;
>                     }
>   }
> {code}
> and it's protected by the synchronized statement. I just remove volatile 
> keyword.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to