[
https://issues.apache.org/jira/browse/ARTEMIS-4714?focusedWorklogId=913091&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-913091
]
ASF GitHub Bot logged work on ARTEMIS-4714:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 04/Apr/24 21:25
Start Date: 04/Apr/24 21:25
Worklog Time Spent: 10m
Work Description: clebertsuconic commented on code in PR #4875:
URL: https://github.com/apache/activemq-artemis/pull/4875#discussion_r1552466732
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/federation/FederatedQueueConsumerImpl.java:
##########
@@ -325,7 +325,9 @@ public void onMessage(ClientMessage clientMessage) {
} catch (Exception e) {
ActiveMQServerLogger.LOGGER.federationDispatchError(clientMessage.toString(),
e);
try {
- clientSession.rollback();
+ if (clientSession != null) {
+ clientSession.rollback();
Review Comment:
in theory the NPE here could still happen.
you check if clientSession != null
a connectionFailed happened on a different thread.
you call rollback on the current thread.
Bang!!!! NPE!!!
Of course the JDK could be using non volatile references.. etc...
But it would be better if you cached the reference locally
Such as
```ClientSession localSession = clientSesssion;
if (localSession != null) {
localSession.rollback();
}
```
I agree the probably of the NPE happening is lower with your changes.. but
they can still happen.
Issue Time Tracking
-------------------
Worklog Id: (was: 913091)
Time Spent: 20m (was: 10m)
> Mitigate NPE in FederatedQueueConsumerImpl MessageListener
> ----------------------------------------------------------
>
> Key: ARTEMIS-4714
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4714
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Reporter: Justin Bertram
> Assignee: Justin Bertram
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
