franz1981 commented on a change in pull request #2577: ARTEMIS-1604 Artemis
deadlock using MQTT Protocol
URL: https://github.com/apache/activemq-artemis/pull/2577#discussion_r264572694
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
##########
@@ -249,7 +253,7 @@
private volatile boolean directDeliver = true;
- private volatile boolean supportsDirectDeliver = true;
+ private volatile boolean supportsDirectDeliver = false;
Review comment:
@michaelandrepearce @clebertsuconic
According to @FrederikP it is an actual issue and with MQTT + debugging I've
been able to reproduce it with ease: I just need to do the same using a test
(and probably ByteMan or ByteBuddy to make it happen).
Re the why it can be fixed is not ease: according to what Clebert said I
believe him that there are protocols that do not support direct delivery and
MQTT is reporting (on the code) to support it.
The simplest fix here is contained in one of the commit of this PR: just
making it to report that it doesn't support direct delivery.
Thanks to the way MQTT works I have found that direct deliveries and async
deliveries could happen concurrently (and TBH it doesn't make sense, from many
point of views) AND that if the locks of ServerConsumerImpl and QueueImpl are
not acquired in the correct order it could lead to a deadlock.
Given that async and direct deliveries shouldn't happen concurrently (are
mutually exclusive, but this wasn't never enforced on code level!), fixing it
will make irrelevent the order of locks.
Now I'm checking if Core is exposed to the same issue.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
