This is an automated email from the ASF dual-hosted git repository. cshannon pushed a commit to branch 6.1.x.AMF.x in repository https://gitbox.apache.org/repos/asf/activemq.git
commit c3f39a5e66cbe955a4dc816d0e8fdc60411814a6 Author: Christopher L. Shannon <[email protected]> AuthorDate: Wed Sep 25 10:00:15 2024 -0400 AMFU-120 - Fix potential deadlock stopping stuck connection When listing connection states get rid of the transport lock as the connection state tracker is now fully protected by its own lock. This should help prevent deadlocks when the inactivity monitor is trying to stop a stuck connection and can't get the transport lock when listing the connection states during logging or asyncStop because the nio handler still has the lock. AMF-3537 (cherry picked from commit e54887cd5abb0e0dee03cc976a2acfeaf4049abc) --- .../broker/SingleTransportConnectionStateRegister.java | 15 ++++++++++----- .../org/apache/activemq/broker/TransportConnection.java | 9 ++++++++- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/SingleTransportConnectionStateRegister.java b/activemq-broker/src/main/java/org/apache/activemq/broker/SingleTransportConnectionStateRegister.java index 159c6e6d09..dfdd5a1cc6 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/SingleTransportConnectionStateRegister.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/SingleTransportConnectionStateRegister.java @@ -29,7 +29,8 @@ import org.apache.activemq.command.ProducerId; import org.apache.activemq.command.SessionId; /** - * + * @AMF_CHANGE - AMFU-120 - Add missing synchronized keywords to methods + * Eventually this whole class should be modernized */ public class SingleTransportConnectionStateRegister implements TransportConnectionStateRegister{ @@ -37,7 +38,8 @@ public class SingleTransportConnectionStateRegister implements TransportConnect private TransportConnectionState connectionState; private ConnectionId connectionId; - public TransportConnectionState registerConnectionState(ConnectionId connectionId, + // @AMF_CHANGE - Add missing synchronized. + public synchronized TransportConnectionState registerConnectionState(ConnectionId connectionId, TransportConnectionState state) { TransportConnectionState rc = connectionState; connectionState = state; @@ -120,7 +122,8 @@ public class SingleTransportConnectionStateRegister implements TransportConnect return connectionState == null; } - public void intialize(TransportConnectionStateRegister other) { + // @AMF_CHANGE - Add missing synchronized. + public synchronized void intialize(TransportConnectionStateRegister other) { if (other.isEmpty()){ clear(); @@ -134,7 +137,8 @@ public class SingleTransportConnectionStateRegister implements TransportConnect } - public Map<ConnectionId, TransportConnectionState> mapStates() { + // @AMF_CHANGE - Add missing synchronized. + public synchronized Map<ConnectionId, TransportConnectionState> mapStates() { Map<ConnectionId, TransportConnectionState> map = new HashMap<ConnectionId, TransportConnectionState>(); if (!isEmpty()) { map.put(connectionId, connectionState); @@ -142,7 +146,8 @@ public class SingleTransportConnectionStateRegister implements TransportConnect return map; } - public void clear() { + // @AMF_CHANGE - Add missing synchronized. + public synchronized void clear() { connectionState=null; connectionId=null; diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java b/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java index 0bf43a549e..3b3b2e4f02 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java +++ b/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java @@ -1707,9 +1707,16 @@ public class TransportConnection implements Connection, Task, CommandVisitor { return connectionStateRegister.unregisterConnectionState(connectionId); } - protected synchronized List<TransportConnectionState> listConnectionStates() { + // @AMF_CHANGE - AMFU-120 - don't need to synchronize as the single version already syncs + // and the multi version uses a concurrent hash map. Both versions copy the values into + // a new array list when returning. Removing this sync on the transport connection helps + // prevent potential deadlocks when the connection is stuck starting and an inactivity + // monitor or other process tries to stop the connection but can't get the transport lock + // because this method call is used during stop() and exception logging + protected List<TransportConnectionState> listConnectionStates() { return connectionStateRegister.listConnectionStates(); } + // END @AMF_CHANGE protected synchronized TransportConnectionState lookupConnectionState(String connectionId) { return connectionStateRegister.lookupConnectionState(connectionId); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information, visit: https://activemq.apache.org/contact
