[
https://issues.apache.org/jira/browse/ARTEMIS-4771?focusedWorklogId=919982&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-919982
]
ASF GitHub Bot logged work on ARTEMIS-4771:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 20/May/24 09:16
Start Date: 20/May/24 09:16
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #4932:
URL: https://github.com/apache/activemq-artemis/pull/4932#discussion_r1606489945
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/AMQPLargeMessageWriter.java:
##########
@@ -170,16 +170,26 @@ private void resume() {
}
private void tryDelivering() {
+
+ final Delivery localDelivery = delivery;
+ final MessageReference localReference = reference;
+ final LargeBodyReader localBodyReader = largeBodyReader;
+
+ if (localDelivery == null || localReference == null || localBodyReader
== null) {
+ logger.debug("Write got closed before tryDelivering was called");
+ return;
+ }
Review Comment:
The close method is only safe to call on the connection thread, which is
where it appears to be being called. Are you seeing otherwise? If not there is
no benefit in introducing the 3 additional variables and complicating the code
with a huge if statement (might even mislead folks into thinking there is some
safety there isnt). Should be no need for synchronization as the writing code
is not called concurrently. It would seem its simply running from a previous
scheduling, but no longer needs to. If so that can be addressed by checking
'closed' at the start of the scheduled task if it was closed already before
proceeding.
Issue Time Tracking
-------------------
Worklog Id: (was: 919982)
Time Spent: 2h 20m (was: 2h 10m)
> NPE between AMQPLargeMessageWriter::tryDelivering and resetClose
> ----------------------------------------------------------------
>
> Key: ARTEMIS-4771
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4771
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Reporter: Clebert Suconic
> Priority: Major
> Time Spent: 2h 20m
> Remaining Estimate: 0h
>
> This is using RedHat's bits:
> java.lang.NullPointerException: Cannot invoke
> "org.apache.qpid.proton.engine.Delivery.getTag()" because "this.delivery" is
> null
> at
> org.apache.activemq.artemis.protocol.amqp.proton.AMQPLargeMessageWriter.tryDelivering(AMQPLargeMessageWriter.java:174)
> ~[artemis-amqp-protocol-2.33.0.redhat-00009.jar:2.33.0.redhat-00009]
> at
> io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:173)
> ~[netty-common-4.1.108.Final-redhat-00001.jar:4.1.108.Final-redhat-00001]
> at
> io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:166)
> [netty-common-4.1.108.Final-redhat-00001.jar:4.1.108.Final-redhat-00001]
> at
> io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)
> [netty-common-4.1.108.Final-redhat-00001.jar:4.1.108.Final-redhat-00001]
> at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:413)
> [netty-transport-classes-epoll-4.1.108.Final-redhat-00001.jar:4.1.108.Final-redhat-00001]
> at
> io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
> [netty-common-4.1.108.Final-redhat-00001.jar:4.1.108.Final-redhat-00001]
> at
> io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
> [netty-common-4.1.108.Final-redhat-00001.jar:4.1.108.Final-redhat-00001]
> at
> org.apache.activemq.artemis.utils.ActiveMQThreadFactory$1.run(ActiveMQThreadFactory.java:118)
> [artemis-commons-2.33.0.redhat-00009.jar:2.33.0.redhat-00009]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
