nonbinaryprogrammer commented on a change in pull request #7471:
URL: https://github.com/apache/geode/pull/7471#discussion_r832677649
##########
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:
There is the possibility for the `queueConnections` to be null during
teardown of `CacheClientNotifierDUnitTest`. While the cache is closing and
everything is being freed, the `RedundancySatisfierTask` continues to check
`queueConnections` whenever it is referenced (because it is volatile). So if
the `RedundancySatisfierTask` thread is slow to close and is still checking the
`queueConnections` after it has been freed, it could cause an NPE.
Do you think that a test for that situation would be valuable? I worry that
it will just be forcing weird timing, and would probably be flaky.
--
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]
