[
https://issues.apache.org/jira/browse/ZOOKEEPER-1739?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13736159#comment-13736159
]
qingjie qiao commented on ZOOKEEPER-1739:
-----------------------------------------
Not sure if this is really a problem or we should just ignore this. If it is,
threre are a lot more similar codes. Since I'm new to zk and haven't read all
the codes, the patch may not cover all the codes with this problem.
make the workerPool visible to SelectorThread:
{code}
Index: src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java
===================================================================
--- src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java (版本
1512570)
+++ src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java (工作副本)
@@ -722,11 +722,11 @@
@Override
public void start() {
- stopped = false;
if (workerPool == null) {
workerPool = new WorkerService(
"NIOWorker", numWorkerThreads, false);
}
+ stopped = false;
for(SelectorThread thread : selectorThreads) {
if (thread.getState() == Thread.State.NEW) {
thread.start();
{code}
make cnxTO visible to QuorumCnxManager.Listener thread, SendWorker's fields
visible to SendWorker thread and RecvWorker's fields visible to RecvWorker:
{code}
Index: src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
===================================================================
--- src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
(版本 1512570)
+++ src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
(工作副本)
@@ -85,7 +85,7 @@
* Connection time out value in milliseconds
*/
- private int cnxTO = 5000;
+ private volatile int cnxTO = 5000;
/*
* Local IP address
@@ -593,11 +593,11 @@
* one.
*/
class SendWorker extends Thread {
- Long sid;
- Socket sock;
+ final Long sid;
+ final Socket sock;
RecvWorker recvWorker;
volatile boolean running = true;
- DataOutputStream dout;
+ volatile DataOutputStream dout;
/**
* An instance of this thread receives messages to send
@@ -747,10 +747,10 @@
* channel breaks, then removes itself from the pool of receivers.
*/
class RecvWorker extends Thread {
- Long sid;
- Socket sock;
+ final Long sid;
+ final Socket sock;
volatile boolean running = true;
- DataInputStream din;
+ volatile DataInputStream din;
final SendWorker sw;
RecvWorker(Socket sock, Long sid, SendWorker sw) {
{code}
make manager visible to WorkerReceiver and WorkerSender thread:
{code}
Index: src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java
===================================================================
--- src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java
(版本 1512570)
+++ src/java/main/org/apache/zookeeper/server/quorum/FastLeaderElection.java
(工作副本)
@@ -201,7 +201,7 @@
class WorkerReceiver implements Runnable {
volatile boolean stop;
- QuorumCnxManager manager;
+ final QuorumCnxManager manager;
WorkerReceiver(QuorumCnxManager manager) {
this.stop = false;
@@ -410,7 +410,7 @@
class WorkerSender implements Runnable {
volatile boolean stop;
- QuorumCnxManager manager;
+ final QuorumCnxManager manager;
WorkerSender(QuorumCnxManager manager){
this.stop = false;
{code}
make quorumVerifier visible to QuorumPeer thread and all initialized fields
visible to QuorumPeer thread
{code}
Index: src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java
===================================================================
--- src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java (版本
1512570)
+++ src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java (工作副本)
@@ -342,7 +342,7 @@
*/
//last committed quorum verifier
- public QuorumVerifier quorumVerifier;
+ public volatile QuorumVerifier quorumVerifier;
//last proposed quorum verifier
public QuorumVerifier lastSeenQuorumVerifier = null;
@@ -603,6 +603,7 @@
loadDataBase();
cnxnFactory.start();
startLeaderElection();
+ running = true;
super.start();
}
{code}
> 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
> Assignee: 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
