tabish121 commented on code in PR #5346:
URL: https://github.com/apache/activemq-artemis/pull/5346#discussion_r1842971295
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonAbstractReceiver.java:
##########
@@ -346,6 +346,7 @@ public void onMessageComplete(Delivery delivery,
connection.requireInHandler();
try {
+ this.messageReader.close();
Review Comment:
I'd like a clear explanation of what the **actual** issue you are fixing is
as you still haven't made it in any way clear, just a "race", a race on what?
What is the outcome of the race that is apparently the root of the bug? How is
the bug expressed? Totally not yet clear.
Then some explanation of how that close connection call you pointed to
interacts here as that is on the connection thread is it not? Which is the
same thread the onMessageComplete will be running on right? In which case the
close in the finally would have run and done nothing special since
AMQPLargeMessageReader already called release resources on the large message
body instance and cleared the currentMessage field (which is volatile so it is
truly cleared).
Calling advance doesn't actually trigger anything itself in this context, it
just modifies the proton engine state which won't get acted upon until the next
time you poll the event queue at the most so it isn't clear again how that
matters in this case.
--
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: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
