[ https://issues.apache.org/jira/browse/ZOOKEEPER-1739?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Flavio Junqueira updated ZOOKEEPER-1739: ---------------------------------------- Description: I am reading the trunk source code recently and find a thread-safe problem, but i'm not quite sure. in FastLeaderElection: {code} class WorkerSender implements Runnable { volatile boolean stop; QuorumCnxManager manager; WorkerSender(QuorumCnxManager manager){ this.stop = false; this.manager = manager; } public void run() { ... } } ... Messenger(QuorumCnxManager manager) { this.ws = new WorkerSender(manager); Thread t = new Thread(this.ws, "WorkerSender[myid=" + self.getId() + "]"); t.setDaemon(true); t.start(); this.wr = new WorkerReceiver(manager); t = new Thread(this.wr, "WorkerReceiver[myid=" + self.getId() + "]"); t.setDaemon(true); t.start(); } ... {code} The instance of WorkerSender is constructed in main thread, and its field manager is assigned , and it is used in another thread. The later thread may see that WorkerSender.manager is the default value null. The solution may be: (1) change {code} WorkerSender(QuorumCnxManager manager){ this.stop = false; this.manager = manager; } {code} to {code} WorkerSender(QuorumCnxManager manager){ this.manager = manager; this.stop = false; } {code} or(2) change {code} QuorumCnxManager manager; {code} to {code} final QuorumCnxManager manager; {code} was: I am reading the trunk source code recently and find a thread-safe problem, but i'm not quite sure. in FastLeaderElection: class WorkerSender implements Runnable { volatile boolean stop; QuorumCnxManager manager; WorkerSender(QuorumCnxManager manager){ this.stop = false; this.manager = manager; } public void run() { ... } } ... Messenger(QuorumCnxManager manager) { this.ws = new WorkerSender(manager); Thread t = new Thread(this.ws, "WorkerSender[myid=" + self.getId() + "]"); t.setDaemon(true); t.start(); this.wr = new WorkerReceiver(manager); t = new Thread(this.wr, "WorkerReceiver[myid=" + self.getId() + "]"); t.setDaemon(true); t.start(); } ... The instance of WorkerSender is constructed in main thread, and its field manager is assigned , and it is used in another thread. The later thread may see that WorkerSender.manager is the default value null. The solution may be: (1) change WorkerSender(QuorumCnxManager manager){ this.stop = false; this.manager = manager; } to WorkerSender(QuorumCnxManager manager){ this.manager = manager; this.stop = false; } or(2) change QuorumCnxManager manager; to final QuorumCnxManager manager; > thread safe bug in FastLeaderElection: instance of WorkerSender is not safe > published, WorkerSender thread may see that WorkerSender.manager is the > default value null > ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- > > Key: ZOOKEEPER-1739 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1739 > Project: ZooKeeper > Issue Type: Bug > Components: leaderElection > Affects Versions: 3.4.5 > Reporter: qingjie qiao > Priority: Minor > > I am reading the trunk source code recently and find a thread-safe problem, > but i'm not quite sure. > in FastLeaderElection: > {code} > class WorkerSender implements Runnable { > volatile boolean stop; > QuorumCnxManager manager; > WorkerSender(QuorumCnxManager manager){ > this.stop = false; > this.manager = manager; > } > public void run() { > ... > } > } > ... > Messenger(QuorumCnxManager manager) { > this.ws = new WorkerSender(manager); > Thread t = new Thread(this.ws, > "WorkerSender[myid=" + self.getId() + "]"); > t.setDaemon(true); > t.start(); > this.wr = new WorkerReceiver(manager); > t = new Thread(this.wr, > "WorkerReceiver[myid=" + self.getId() + "]"); > t.setDaemon(true); > t.start(); > } > ... > {code} > The instance of WorkerSender is constructed in main thread, and its field > manager is assigned , and it is used in another thread. The later thread may > see that WorkerSender.manager is the default value null. The solution may be: > (1) change > {code} > WorkerSender(QuorumCnxManager manager){ > this.stop = false; > this.manager = manager; > } > {code} > to > {code} > WorkerSender(QuorumCnxManager manager){ > this.manager = manager; > this.stop = false; > } > {code} > or(2) > change > {code} > QuorumCnxManager manager; > {code} > to > {code} > final QuorumCnxManager manager; > {code} -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira