From 4aa5683f1510f239d621b8463dbe7a375b1e1948 Mon Sep 17 00:00:00 2001 From: "R.M.Morrien" Date: Mon, 11 Mar 2024 12:26:00 +0100 Subject: [PATCH] Fixes issue #24849 make relevant methods synchronized in LocalTxConnectionEventListener and protect associatedHandles from external clear calls. Fixes issue #24849 make relevant methods synchronized in LocalTxConnectionEventListener and protect associatedHandles from external clear calls. --- .../resource/ConnectorXAResource.java | 41 +++++---- .../enterprise/resource/ResourceHandle.java | 2 +- .../LocalTxConnectionEventListener.java | 83 +++++++++++++----- .../LocalTxConnectionEventListenerTest.java | 87 +++++++++++++++++++ 4 files changed, 169 insertions(+), 44 deletions(-) create mode 100644 appserver/connectors/connectors-runtime/src/test/java/com/sun/enterprise/resource/listener/LocalTxConnectionEventListenerTest.java diff --git a/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/ConnectorXAResource.java b/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/ConnectorXAResource.java index d821fbc7230..33f03c76093 100644 --- a/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/ConnectorXAResource.java +++ b/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/ConnectorXAResource.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2022 Contributors to the Eclipse Foundation + * Copyright (c) 2021, 2024 Contributors to the Eclipse Foundation * Copyright (c) 1997, 2020 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the @@ -21,7 +21,7 @@ import static java.util.logging.Level.SEVERE; import java.util.Map; -import java.util.Set; +import java.util.Map.Entry; import java.util.logging.Logger; import javax.transaction.xa.XAException; @@ -46,8 +46,12 @@ */ public class ConnectorXAResource implements XAResource { - static Logger _logger = LogDomains.getLogger(ConnectorXAResource.class, LogDomains.RSR_LOGGER); + private static Logger _logger = LogDomains.getLogger(ConnectorXAResource.class, LogDomains.RSR_LOGGER); + /** + * userHandle meaning: an object representing the "connection handle for the underlying physical connection". In some + * code also named connectionHandle. + */ private Object userHandle; private ResourceSpec spec; private ClientSecurityInfo info; @@ -120,7 +124,9 @@ public void end(Xid xid, int flags) throws XAException { if (handle != null) { // not needed, just to be sure. ManagedConnection associatedConnection = (ManagedConnection) handle.getResource(); associatedConnection.associateConnection(userHandle); - _logger.log(FINE, "connection_sharing_reset_association", userHandle); + if (_logger.isLoggable(FINE)) { + _logger.log(FINE, "connection_sharing_reset_association", userHandle); + } } } } catch (Exception e) { @@ -227,25 +233,18 @@ private JavaEETransaction getCurrentTransaction() throws SystemException { private void resetAssociation() throws XAException { try { ResourceHandle handle = getResourceHandle(); - - LocalTxConnectionEventListener listerner = (LocalTxConnectionEventListener) handle.getListener(); - // Get all associated Handles and reset their ManagedConnection association. - Map associatedHandles = listerner.getAssociatedHandles(); - if (associatedHandles != null) { - Set userHandles = associatedHandles.entrySet(); - for (Map.Entry userHandleEntry : userHandles) { - ResourceHandle associatedHandle = (ResourceHandle) userHandleEntry.getValue(); - ManagedConnection associatedConnection = (ManagedConnection) associatedHandle.getResource(); - associatedConnection.associateConnection(userHandleEntry.getKey()); - if (_logger.isLoggable(FINE)) { - _logger.log(FINE, "connection_sharing_reset_association", userHandleEntry.getKey()); - } + LocalTxConnectionEventListener listener = (LocalTxConnectionEventListener) handle.getListener(); + + // Clear the associations and Map all associated handles back to their actual Managed Connection. + Map associatedHandles = listener.getAssociatedHandlesAndClearMap(); + for (Entry userHandleEntry : associatedHandles.entrySet()) { + ResourceHandle associatedHandle = (ResourceHandle) userHandleEntry.getValue(); + ManagedConnection associatedConnection = (ManagedConnection) associatedHandle.getResource(); + associatedConnection.associateConnection(userHandleEntry.getKey()); + if (_logger.isLoggable(FINE)) { + _logger.log(FINE, "connection_sharing_reset_association", userHandleEntry.getKey()); } - - // All associated handles are mapped back to their actual Managed Connection. Clear the associations. - associatedHandles.clear(); } - } catch (Exception ex) { handleResourceException(ex); } diff --git a/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/ResourceHandle.java b/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/ResourceHandle.java index fa3baea5d15..98e02488ed9 100644 --- a/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/ResourceHandle.java +++ b/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/ResourceHandle.java @@ -52,7 +52,7 @@ public class ResourceHandle implements com.sun.appserv.connectors.internal.api.R private final long id; private final ClientSecurityInfo info; - private final Object resource; // represents MC + private final Object resource; // represents ManagedConnection private ResourceSpec spec; private XAResource xaRes; private Object userConnection; // represents connection-handle to user diff --git a/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/listener/LocalTxConnectionEventListener.java b/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/listener/LocalTxConnectionEventListener.java index a726b58c7ba..1b7185484bc 100644 --- a/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/listener/LocalTxConnectionEventListener.java +++ b/appserver/connectors/connectors-runtime/src/main/java/com/sun/enterprise/resource/listener/LocalTxConnectionEventListener.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022 Contributors to the Eclipse Foundation + * Copyright (c) 2022, 2024 Contributors to the Eclipse Foundation * Copyright (c) 1997, 2020 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the @@ -32,37 +32,51 @@ */ public class LocalTxConnectionEventListener extends ConnectionEventListener { - private PoolManager poolManager; + /** + * A shortcut to the singleton PoolManager instance. Field could also be removed. + */ + private final PoolManager poolManager = ConnectorRuntime.getRuntime().getPoolManager(); - // connectionHandle -> ResourceHandle - // Whenever a connection is associated with a ManagedConnection, - // that connection and the resourcehandle associated with its - // original ManagedConnection will be put in this table. - private IdentityHashMap associatedHandles; + /** + * Map to store the relation: "userHandle/connectionHandle -> ResourceHandle" using reference-equality. Whenever a + * connection is associated with a ManagedConnection, that connection and the resourceHandle associated with its + * original ManagedConnection will be put in this table. + *

+ * userHandle meaning: an object representing the "connection handle for the underlying physical connection". In some + * code also named connectionHandle. + *

+ * All code altering associatedHandles must be synchronized. + */ + private final IdentityHashMap associatedHandles = new IdentityHashMap<>(10); - private ResourceHandle resource; + /** + * The original resource for which this listener is created. + */ + private final ResourceHandle resource; public LocalTxConnectionEventListener(ResourceHandle resource) { this.resource = resource; - this.associatedHandles = new IdentityHashMap(10); - this.poolManager = ConnectorRuntime.getRuntime().getPoolManager(); } @Override - public void connectionClosed(ConnectionEvent evt) { + public synchronized void connectionClosed(ConnectionEvent evt) { Object connectionHandle = evt.getConnectionHandle(); ResourceHandle handle = resource; if (associatedHandles.containsKey(connectionHandle)) { - handle = (ResourceHandle) associatedHandles.get(connectionHandle); + handle = associatedHandles.get(connectionHandle); } + // ManagedConnection instance is still valid and put back in the pool: do not remove the event listener. poolManager.resourceClosed(handle); } @Override - public void connectionErrorOccurred(ConnectionEvent evt) { + public synchronized void connectionErrorOccurred(ConnectionEvent evt) { resource.setConnectionErrorOccurred(); + + // ManagedConnection instance is now invalid and unusable. Remove this event listener. ManagedConnection mc = (ManagedConnection) evt.getSource(); mc.removeConnectionEventListener(this); + poolManager.resourceErrorOccurred(resource); } @@ -72,12 +86,15 @@ public void connectionErrorOccurred(ConnectionEvent evt) { * @param evt ConnectionEvent */ @Override - public void badConnectionClosed(ConnectionEvent evt) { + public synchronized void badConnectionClosed(ConnectionEvent evt) { Object connectionHandle = evt.getConnectionHandle(); ResourceHandle handle = resource; if (associatedHandles.containsKey(connectionHandle)) { - handle = (ResourceHandle) associatedHandles.get(connectionHandle); + handle = associatedHandles.get(connectionHandle); } + + // TODO: Explain why event listener needs to be removed. + // There is no documentation mentioning: ManagedConnection instance is now invalid and unusable. ManagedConnection mc = (ManagedConnection) evt.getSource(); mc.removeConnectionEventListener(this); @@ -99,16 +116,38 @@ public void localTransactionRolledback(ConnectionEvent evt) { // no-op } - public void associateHandle(Object c, ResourceHandle h) { - associatedHandles.put(c, h); + /** + * Associate the given userHandle to the resourceHandle. + * + * @param userHandle the userHandle object to be associated with the new handle + * @param resourceHandle the original Handle + */ + public synchronized void associateHandle(Object userHandle, ResourceHandle resourceHandle) { + associatedHandles.put(userHandle, resourceHandle); } - public ResourceHandle removeAssociation(Object c) { - return (ResourceHandle) associatedHandles.remove(c); + /** + * Removes the Map entory for the given userHandle key. + * + * @param userHandle The userHandle key to be removed from the map. + * @return the associated ResourceHandle that is removed from the map or null if no association was found. A null return + * can also indicate that the map previously associated null with userHandle. + */ + public synchronized ResourceHandle removeAssociation(Object userHandle) { + return associatedHandles.remove(userHandle); } - public Map getAssociatedHandles() { - return associatedHandles; - } + /** + * Returns a clone of the whole associatedHandles map and clears the map in the listener. + * @return The clone of the associatedHandles map. + */ + public synchronized Map getAssociatedHandlesAndClearMap() { + // Clone the associatedHandles, because we will clear the list in this method + IdentityHashMap result = (IdentityHashMap) associatedHandles.clone(); + // Clear the associatedHandles + associatedHandles.clear(); + + return result; + } } diff --git a/appserver/connectors/connectors-runtime/src/test/java/com/sun/enterprise/resource/listener/LocalTxConnectionEventListenerTest.java b/appserver/connectors/connectors-runtime/src/test/java/com/sun/enterprise/resource/listener/LocalTxConnectionEventListenerTest.java new file mode 100644 index 00000000000..ee02d2a19af --- /dev/null +++ b/appserver/connectors/connectors-runtime/src/test/java/com/sun/enterprise/resource/listener/LocalTxConnectionEventListenerTest.java @@ -0,0 +1,87 @@ +/* + * Copyright (c) 2024 Contributors to the Eclipse Foundation + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0, which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * This Source Code may also be made available under the following Secondary + * Licenses when the conditions for such availability set forth in the + * Eclipse Public License v. 2.0 are satisfied: GNU General Public License, + * version 2 with the GNU Classpath Exception, which is available at + * https://www.gnu.org/software/classpath/license.html. + * + * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 + */ + +package com.sun.enterprise.resource.listener; + +import static org.easymock.EasyMock.createNiceMock; +import static org.easymock.EasyMock.replay; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.util.Map; + +import org.glassfish.api.naming.SimpleJndiName; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import com.sun.appserv.connectors.internal.api.PoolingException; +import com.sun.enterprise.connectors.ConnectorRuntime; +import com.sun.enterprise.resource.ResourceHandle; +import com.sun.enterprise.resource.ResourceSpec; + +import jakarta.resource.ResourceException; +import jakarta.resource.spi.ManagedConnection; + +public class LocalTxConnectionEventListenerTest { + + @BeforeEach + public void setup() throws PoolingException, ResourceException { + // Make sure ConnectorRuntime singleton is initialized + new ConnectorRuntime(); + } + + @Test + public void associateHandleTest() throws ResourceException { + ResourceHandle mainResourceHandle = createResourceHandle(1); + LocalTxConnectionEventListener localTxConnectionEventListener = new LocalTxConnectionEventListener(mainResourceHandle); + + // Associate a new handle association + ResourceHandle associatedResourceHandle = createResourceHandle(2); + final Object userHandle = new Object(); + localTxConnectionEventListener.associateHandle(userHandle, associatedResourceHandle); + + // Remove the new handle association + ResourceHandle removeAssociation = localTxConnectionEventListener.removeAssociation(userHandle); + assertEquals(associatedResourceHandle, removeAssociation); + + // Check the remove did work in the previous call + removeAssociation = localTxConnectionEventListener.removeAssociation(userHandle); + assertNull(removeAssociation); + } + + @Test + public void getAssociatedHandlesAndClearMapTest() throws ResourceException { + ResourceHandle mainResourceHandle = createResourceHandle(1); + LocalTxConnectionEventListener localTxConnectionEventListener = new LocalTxConnectionEventListener(mainResourceHandle); + + localTxConnectionEventListener.associateHandle(new Object(), createResourceHandle(2)); + localTxConnectionEventListener.associateHandle(new Object(), createResourceHandle(3)); + + // Check the clone works + Map associatedHandlesAndClearMap = localTxConnectionEventListener.getAssociatedHandlesAndClearMap(); + assertEquals(2, associatedHandlesAndClearMap.size()); + + // Check the clear did work in the previous call + associatedHandlesAndClearMap = localTxConnectionEventListener.getAssociatedHandlesAndClearMap(); + assertEquals(0, associatedHandlesAndClearMap.size()); + } + + private ResourceHandle createResourceHandle(int i) throws ResourceException { + ManagedConnection managedConnection = createNiceMock(ManagedConnection.class); + replay(); + return new ResourceHandle(managedConnection, new ResourceSpec(new SimpleJndiName("testResource" + i), 0), null, null); + } +}