tabish121 commented on code in PR #4932:
URL: https://github.com/apache/activemq-artemis/pull/4932#discussion_r1602358744
##########
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:
I don't see how not throwing an NPE due to thread synchronization issues
isn't best solved by proper thread synchronization...
With the current fix it could continue writing after the close if the close
is happening on another thread than the write as the values being checked
aren't synced on a barrier so they could be non-null for more than one
execution until a cache flush is forced by some other barrier. It might not NPE
now but it still isn't guaranteed to stop when you intended it to.
--
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]
