Github user shoukunhuai commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1827#discussion_r165333374
  
    --- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/RefsOperation.java
 ---
    @@ -152,14 +153,14 @@ protected void rollbackRedelivery(Transaction tx, 
MessageReference ref, long tim
        @Override
        public void afterCommit(final Transaction tx) {
           for (MessageReference ref : refsToAck) {
    -         synchronized (ref.getQueue()) {
    --- End diff --
    
    I do this because in QueueImpl::postAcknowledge, the only thing that 
changed queue's state is decrement queue's delivering count, which i believe is 
thread safe.
    
    While QueueImpl:postRollback is different, it will not return messages if 
purge on no consumers and there is no consumer at that time, since add/remove 
consumer will lock the queue. So we must lock the queue when do post rollback.
    
    If no body is 100% sure about this, i will revert this change, since other 
change has improved a lot.


---

Reply via email to