albertogpz commented on a change in pull request #7471:
URL: https://github.com/apache/geode/pull/7471#discussion_r834536906
##########
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:
Yes, you did what I described. But, unfortunately, I have realized that
this will not work (see the test cases failing) because the `queueConnections`
is a copy on write structure, meaning that every time it is updated, a fresh
copy is created.
With this change, the `RedundancySatisfierTask` will eventually not point to
the same instance as the `QueueManagerImpl` instance.
The problem with the null check added is that I think it is just one of the
places where it should be added. Plus, you would always be in risk on hitting
the race condition even if you first checked that the `queueConnections` is
null. What if right after you check it is set to null?
How about holding a reference to the `QueueManagerImpl` instance in the
`RedundancySatisfierTask` instance?
You could pass it when you create the `RedundancySatisfierTask` as follows:
`redundancySatisfierTask = new RedundancySatisfierTask(this);`
--
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]
