[QPID-8510] [Broker-J] [AMQP 1.0] Connection transaction management is not thread-safe

• Type: Bug
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: qpid-java-broker-7.0.0, qpid-java-broker-7.0.1, qpid-java-broker-7.1.0, qpid-java-broker-7.0.4, qpid-java-broker-7.0.5, qpid-java-broker-7.0.6, qpid-java-broker-7.0.7, qpid-java-broker-8.0.0, qpid-java-broker-7.1.1, qpid-java-broker-7.1.2, qpid-java-broker-7.0.8, qpid-java-broker-7.1.3, qpid-java-broker-7.1.4, qpid-java-broker-7.0.9, qpid-java-broker-7.1.5, qpid-java-broker-7.1.6, qpid-java-broker-7.1.7, qpid-java-broker-7.1.8, qpid-java-broker-8.0.1, qpid-java-broker-7.1.9, qpid-java-broker-8.0.2, qpid-java-broker-7.1.10, qpid-java-broker-8.0.3, qpid-java-broker-7.1.11, qpid-java-broker-8.0.4, qpid-java-broker-7.1.12
• Fix Version/s: qpid-java-broker-8.0.5
• Component/s: Broker-J
• Labels: Broker Java
• Assignee: Unassigned
• Reporter: Marek Laca
• Created: 12/Mar/21 16:50
• Updated: 20/Jun/21 11:41
• Resolved: 08/May/21 20:29

Description

Based on Java documentation a change of volatile variable is always visible to other threads. Hence, assignment a new array to the volatile variable guarantees the visibility of the new array to another threads, but there is not any guarantee of the visibility of a new element of the array. Because the insertion of a new element into the volatile array is a change of the internal state of the array.
For example there is the method AMQPConnection_1_0Impl::removeTransaction:

    private volatile ServerTransaction[] _openTransactions = new ServerTransaction[16];

    @Override
    public void removeTransaction(final int txnId)
    {
        try
        {
            _openTransactions[txnId] = null; // There is not any  guarantee of the visibility, when the change is propagated to another threads.
        }
        catch (ArrayIndexOutOfBoundsException e)
        {
            throw new UnknownTransactionException(txnId);
        }
    }

The same issue is in other methods of AMQPConnection_1_0Impl class.

A concurrent collection can be used instead of the volatile array.