Github user arrodrigues commented on a diff in the pull request:
https://github.com/apache/curator/pull/262#discussion_r179652355
--- Diff:
curator-framework/src/main/java/org/apache/curator/framework/state/ConnectionStateManager.java
---
@@ -340,4 +338,12 @@ private void setCurrentConnectionState(ConnectionState
newConnectionState)
currentConnectionState = newConnectionState;
startOfSuspendedEpoch = (currentConnectionState ==
ConnectionState.SUSPENDED) ? System.currentTimeMillis() : 0;
}
+
+ private synchronized int getUseSessionTimeoutMs() {
--- End diff --
Yes. I had this discussion with @cammckenzie some comments ago. Every
access to startOfSuspendedEpoch is synchronized on "this" except this point, we
could make it volatile or sync this point more, so we choose to sync it. In the
first version I synchronized just the line accessing startOfSuspendedEpoch, but
after adding getUseSessionTimeoutMs I tought it'd be more readable sync the
whole method.
Do you prefer to make it volatile?
---
