maoling edited a comment on issue #1257: ZOOKEEPER-3652: Synchronize ClientCnxn 
outgoing queue flush on a stable internal value
URL: https://github.com/apache/zookeeper/pull/1257#issuecomment-593204387
 
 
   looks good. I still have another thought(no binding for the following, you 
can ignore it):
   - Since the root cause is a narrow windows between `queuePacket` and 
`cleanup`, so synchronized `objectLock` is also an alternative way? which one 
is better? Since `outgoingQueue` is a critical Queue for client to talk with 
server, synchronized `outgoingQueue` will have performance and future program 
extensibility issue?
   
   Haha, I also test for the `global` and `inner` wording by the following way:
   - new two different zookeeper clients, create some znodes, printing the 
`hashcode` of `outgoingQueue` and `state`. They really hold different 
`outgoingQueue`, but the same hashcode of `state`. it really confuses me.
   
   - I believe different clients will have different `state` instance, 
otherwise when one client calls `close()`(set `state` to `CLOSE`), it will 
affect another client. However, using following ways cannot reason about it.
   ```
   System.out.println(zk1.getState() == zk2.getState()); //true
   System.out.println(zk1.getState().equals(zk2.getState())); //true
   System.out.println(zk1.getState().hashCode() == zk2.getState().hashCode()); 
//true
   System.out.println(System.identityHashCode(zk1.getState()) == 
System.identityHashCode(zk2.getState().hashCode()); //true
   ```
   
   `javap` to see the bytecode, the value set to `state` is `public static 
final`, so it's really global-shared by multi-clients.
   ```
   public final class org.apache.zookeeper.States extends 
java.lang.Enum<org.apache.zookeeper.States> {
     public static final org.apache.zookeeper.States CONNECTING;
     public static final org.apache.zookeeper.States NOT_CONNECTED;
     ********************************************
     private static final org.apache.zookeeper.States[] $VALUES;
     public static org.apache.zookeeper.States[] values();
   ```
   
   - synchronized `Enum` is also not thread-safe. Look at my demo attached in 
JIRA.
   - In a word, `Enum` is a heresy:)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to