[GitHub] [geode] boglesby commented on a change in pull request #5750: GEODE-8706: getConnection should get readLock to sync with destroyCon…

[GitBox <gi...@apache.org>]

boglesby commented on a change in pull request #5750:
URL: https://github.com/apache/geode/pull/5750#discussion_r524456655



##########
File path: geode-wan/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventRemoteDispatcher.java
##########
@@ -295,9 +301,16 @@ public Connection getConnection(boolean startAckReaderThread) throws GatewaySend
     // OR the connection's ServerLocation doesn't match with the one stored in sender
     // THEN initialize the connection
     if (!this.sender.isParallel()) {
-      if (this.connection == null || this.connection.isDestroyed()
-          || this.connection.getServer() == null
-          || !this.connection.getServer().equals(this.sender.getServerLocation())) {
+      boolean needToReconnect = false;
+      getConnectionLifeCycleLock().readLock().lock();
+      try {
+        needToReconnect = this.connection == null || this.connection.isDestroyed()
+            || this.connection.getServer() == null
+            || !this.connection.getServer().equals(this.sender.getServerLocation());
+      } finally {
+        getConnectionLifeCycleLock().readLock().unlock();
+      }
+      if (needToReconnect) {
         if (logger.isDebugEnabled()) {

Review comment:
       Does this debug statement need to be moved inside the readLock too since it calls `this.connection.getServer()`?




----------------------------------------------------------------
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:
users@infra.apache.org