albertogpz commented on a change in pull request #7471:
URL: https://github.com/apache/geode/pull/7471#discussion_r833540481



##########
File path: 
geode-core/src/main/java/org/apache/geode/cache/client/internal/QueueManagerImpl.java
##########
@@ -853,7 +853,8 @@ void recoverPrimary(Set<ServerLocation> excludedServers) {
       return;
     }
     final boolean isDebugEnabled = logger.isDebugEnabled();
-    if (queueConnections.getPrimary() != null && 
!queueConnections.getPrimary().isDestroyed()) {
+    if (queueConnections != null && queueConnections.getPrimary() != null
+        && !queueConnections.getPrimary().isDestroyed()) {

Review comment:
       So you mean that the `QueueManagerImpl` instance has been freed (and in 
turn the `queueConnections` member variable) by the cache closing and the 
`RedundancySatisfierTask` thread is still accessing `queueConnections`?
   
   If that could happen I see other possible trouble points as this thread is 
accessing in the `recoverPrimary()` method other members of the 
`QueueManagerImpl` like `pool` after the check you have added and also calling 
other methods of `queueConnections`.
   
   What if the `RedundancySatisfierTask` had an instance member holding a 
reference to the `QueueManagerImpl` instance (passed in the constructor)? In 
that case, even if when the cache is closed the `QueueManagerImpl` instance is 
set to null, it will not be freed (yet) because the `RedundancySatisfierTask` 
still holds a reference to it.
   




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to