Repository: commons-dbcp Updated Branches: refs/heads/master 9bc78d88a -> c19d7950c
Redo JMX internals to use a wrapper that acts as a NOOP is JMX is not available. Project: http://git-wip-us.apache.org/repos/asf/commons-dbcp/repo Commit: http://git-wip-us.apache.org/repos/asf/commons-dbcp/commit/c19d7950 Tree: http://git-wip-us.apache.org/repos/asf/commons-dbcp/tree/c19d7950 Diff: http://git-wip-us.apache.org/repos/asf/commons-dbcp/diff/c19d7950 Branch: refs/heads/master Commit: c19d7950cbc59f03865225865b44048dfa193080 Parents: 9bc78d8 Author: Gary Gregory <ggreg...@apache.org> Authored: Tue Jan 16 14:10:57 2018 -0700 Committer: Gary Gregory <ggreg...@apache.org> Committed: Tue Jan 16 14:10:57 2018 -0700 ---------------------------------------------------------------------- .../apache/commons/dbcp2/BasicDataSource.java | 42 ++--- .../apache/commons/dbcp2/ObjectNameWrapper.java | 92 +++++++++++ .../commons/dbcp2/PoolableConnection.java | 161 +++++++++---------- 3 files changed, 180 insertions(+), 115 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/c19d7950/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java index 8ceecbe..5c9d3ce 100644 --- a/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java +++ b/src/main/java/org/apache/commons/dbcp2/BasicDataSource.java @@ -38,7 +38,6 @@ import java.util.Set; import java.util.logging.Logger; import javax.management.InstanceAlreadyExistsException; -import javax.management.JMException; import javax.management.MBeanRegistration; import javax.management.MBeanRegistrationException; import javax.management.MBeanServer; @@ -1936,14 +1935,7 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean @Override public synchronized void close() throws SQLException { if (registeredJmxObjectName != null) { - final MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); - if (mbs.isRegistered(registeredJmxObjectName)) { - try { - mbs.unregisterMBean(registeredJmxObjectName); - } catch (final JMException e) { - log.warn("Failed to unregister the JMX name: " + registeredJmxObjectName, e); - } - } + registeredJmxObjectName.unregisterMBean(); registeredJmxObjectName = null; } closed = true; @@ -2304,7 +2296,7 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean final ConnectionFactory driverConnectionFactory) throws SQLException { PoolableConnectionFactory connectionFactory = null; try { - connectionFactory = new PoolableConnectionFactory(driverConnectionFactory, registeredJmxObjectName); + connectionFactory = new PoolableConnectionFactory(driverConnectionFactory, registeredJmxObjectName.unwrap()); connectionFactory.setValidationQuery(validationQuery); connectionFactory.setValidationQueryTimeout(validationQueryTimeout); connectionFactory.setConnectionInitSql(connectionInitSqls); @@ -2357,7 +2349,7 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean /** * Actual name under which this component has been registered. */ - private ObjectName registeredJmxObjectName = null; + private ObjectNameWrapper registeredJmxObjectName; private void jmxRegister() { // Return immediately if this DataSource has already been registered @@ -2369,21 +2361,10 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean if (requestedName == null) { return; } - ObjectName oname; - try { - oname = new ObjectName(requestedName); - } catch (final MalformedObjectNameException e) { - log.warn("The requested JMX name [" + requestedName + - "] was not valid and will be ignored."); - return; - } - - final MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); try { - mbs.registerMBean(this, oname); - } catch (InstanceAlreadyExistsException | MBeanRegistrationException - | NotCompliantMBeanException e) { - log.warn("Failed to complete JMX registration", e); + ObjectNameWrapper.wrap(requestedName).registerMBean(); + } catch (MalformedObjectNameException e) { + log.warn("The requested JMX name [" + requestedName + "] was not valid and will be ignored."); } } @@ -2392,16 +2373,15 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean final String requestedName = getJmxName(); if (requestedName != null) { try { - registeredJmxObjectName = new ObjectName(requestedName); + registeredJmxObjectName = ObjectNameWrapper.wrap(requestedName); } catch (final MalformedObjectNameException e) { - log.warn("The requested JMX name [" + requestedName + - "] was not valid and will be ignored."); + log.warn("The requested JMX name [" + requestedName + "] was not valid and will be ignored."); } } if (registeredJmxObjectName == null) { - registeredJmxObjectName = objectName; + registeredJmxObjectName = ObjectNameWrapper.wrap(objectName); } - return registeredJmxObjectName; + return registeredJmxObjectName.unwrap(); } @Override @@ -2430,7 +2410,7 @@ public class BasicDataSource implements DataSource, BasicDataSourceMXBean, MBean } protected ObjectName getRegisteredJmxName() { - return registeredJmxObjectName; + return registeredJmxObjectName.unwrap(); } /** http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/c19d7950/src/main/java/org/apache/commons/dbcp2/ObjectNameWrapper.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/dbcp2/ObjectNameWrapper.java b/src/main/java/org/apache/commons/dbcp2/ObjectNameWrapper.java new file mode 100644 index 0000000..b7b1248 --- /dev/null +++ b/src/main/java/org/apache/commons/dbcp2/ObjectNameWrapper.java @@ -0,0 +1,92 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.commons.dbcp2; + +import java.lang.management.ManagementFactory; + +import javax.management.MBeanServer; +import javax.management.MalformedObjectNameException; +import javax.management.ObjectName; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +/** + * Internal wrapper class that allows JMX to be a noop if absent or disabled. + * + * @since 2.2.1 + */ +class ObjectNameWrapper { + + private static final Log log = LogFactory.getLog(ObjectNameWrapper.class); + + private static MBeanServer MBEAN_SERVER = getPlatformMBeanServer(); + + private static MBeanServer getPlatformMBeanServer() { + try { + return ManagementFactory.getPlatformMBeanServer(); + } catch (LinkageError | Exception e) { + // ignore - JMX not available + log.debug("Failed to get platform MBeanServer", e); + return null; + } + } + + public static ObjectNameWrapper wrap(final ObjectName objectName) { + return new ObjectNameWrapper(objectName); + } + + public static ObjectNameWrapper wrap(final String name) throws MalformedObjectNameException { + return wrap(new ObjectName(name)); + } + + private final ObjectName objectName; + + public ObjectNameWrapper(final ObjectName objectName) { + this.objectName = objectName; + } + + public void registerMBean() { + if (MBEAN_SERVER == null) { + return; + } + try { + MBEAN_SERVER.registerMBean(this, objectName); + } catch (LinkageError | Exception e) { + log.warn("Failed to complete JMX registration for " + objectName, e); + } + } + + public void unregisterMBean() { + if (MBEAN_SERVER == null) { + return; + } + if (MBEAN_SERVER.isRegistered(objectName)) { + try { + MBEAN_SERVER.unregisterMBean(objectName); + } catch (LinkageError | Exception e) { + log.warn("Failed to complete JMX unregistration for " + objectName, e); + } + } + } + + public ObjectName unwrap() { + return objectName; + } + +} http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/c19d7950/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java b/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java index 1a0d253..bc77677 100644 --- a/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java +++ b/src/main/java/org/apache/commons/dbcp2/PoolableConnection.java @@ -33,17 +33,15 @@ import javax.management.ObjectName; import org.apache.commons.pool2.ObjectPool; /** - * A delegating connection that, rather than closing the underlying - * connection, returns itself to an {@link ObjectPool} when - * closed. + * A delegating connection that, rather than closing the underlying connection, returns itself to an {@link ObjectPool} + * when closed. * * @author Rodney Waldhoff * @author Glenn L. Nielsen * @author James House * @since 2.0 */ -public class PoolableConnection extends DelegatingConnection<Connection> - implements PoolableConnectionMXBean { +public class PoolableConnection extends DelegatingConnection<Connection> implements PoolableConnectionMXBean { private static MBeanServer MBEAN_SERVER = null; @@ -58,7 +56,7 @@ public class PoolableConnection extends DelegatingConnection<Connection> /** The pool to which I should return. */ private final ObjectPool<PoolableConnection> _pool; - private final ObjectName _jmxObjectName; + private final ObjectNameWrapper _jmxObjectName; // Use a prepared statement for validation, retaining the last used SQL to // check if the validation query has changed. @@ -66,15 +64,14 @@ public class PoolableConnection extends DelegatingConnection<Connection> private String lastValidationSql = null; /** - * Indicate that unrecoverable SQLException was thrown when using this connection. - * Such a connection should be considered broken and not pass validation in the future. + * Indicate that unrecoverable SQLException was thrown when using this connection. Such a connection should be + * considered broken and not pass validation in the future. */ private boolean _fatalSqlExceptionThrown = false; /** - * SQL_STATE codes considered to signal fatal conditions. Overrides the - * defaults in {@link Utils#DISCONNECTION_SQL_CODES} (plus anything starting - * with {@link Utils#DISCONNECTION_SQL_CODE_PREFIX}). + * SQL_STATE codes considered to signal fatal conditions. Overrides the defaults in + * {@link Utils#DISCONNECTION_SQL_CODES} (plus anything starting with {@link Utils#DISCONNECTION_SQL_CODE_PREFIX}). */ private final Collection<String> _disconnectionSqlCodes; @@ -83,43 +80,49 @@ public class PoolableConnection extends DelegatingConnection<Connection> /** * - * @param conn my underlying connection - * @param pool the pool to which I should return when closed - * @param jmxObjectName JMX name - * @param disconnectSqlCodes SQL_STATE codes considered fatal disconnection errors - * @param fastFailValidation true means fatal disconnection errors cause subsequent - * validations to fail immediately (no attempt to run query or isValid) + * @param conn + * my underlying connection + * @param pool + * the pool to which I should return when closed + * @param jmxObjectName + * JMX name + * @param disconnectSqlCodes + * SQL_STATE codes considered fatal disconnection errors + * @param fastFailValidation + * true means fatal disconnection errors cause subsequent validations to fail immediately (no attempt to + * run query or isValid) */ - public PoolableConnection(final Connection conn, - final ObjectPool<PoolableConnection> pool, final ObjectName jmxObjectName, final Collection<String> disconnectSqlCodes, + public PoolableConnection(final Connection conn, final ObjectPool<PoolableConnection> pool, + final ObjectName jmxObjectName, final Collection<String> disconnectSqlCodes, final boolean fastFailValidation) { super(conn); _pool = pool; - _jmxObjectName = jmxObjectName; + _jmxObjectName = ObjectNameWrapper.wrap(jmxObjectName); _disconnectionSqlCodes = disconnectSqlCodes; _fastFailValidation = fastFailValidation; if (jmxObjectName != null) { try { MBEAN_SERVER.registerMBean(this, jmxObjectName); - } catch (InstanceAlreadyExistsException | - MBeanRegistrationException | NotCompliantMBeanException e) { + } catch (InstanceAlreadyExistsException | MBeanRegistrationException | NotCompliantMBeanException e) { // For now, simply skip registration } } } /** - * - * @param conn my underlying connection - * @param pool the pool to which I should return when closed - * @param jmxName JMX name - */ - public PoolableConnection(final Connection conn, - final ObjectPool<PoolableConnection> pool, final ObjectName jmxName) { - this(conn, pool, jmxName, null, false); - } - + * + * @param conn + * my underlying connection + * @param pool + * the pool to which I should return when closed + * @param jmxName + * JMX name + */ + public PoolableConnection(final Connection conn, final ObjectPool<PoolableConnection> pool, + final ObjectName jmxName) { + this(conn, pool, jmxName, null, false); + } @Override protected void passivate() throws SQLException { @@ -127,14 +130,12 @@ public class PoolableConnection extends DelegatingConnection<Connection> setClosedInternal(true); } - /** * {@inheritDoc} * <p> - * This method should not be used by a client to determine whether or not a - * connection should be return to the connection pool (by calling - * {@link #close()}). Clients should always attempt to return a connection - * to the pool once it is no longer required. + * This method should not be used by a client to determine whether or not a connection should be return to the + * connection pool (by calling {@link #close()}). Clients should always attempt to return a connection to the pool + * once it is no longer required. */ @Override public boolean isClosed() throws SQLException { @@ -153,11 +154,10 @@ public class PoolableConnection extends DelegatingConnection<Connection> return false; } - /** * Returns me to my pool. */ - @Override + @Override public synchronized void close() throws SQLException { if (isClosedInternal()) { // already closed @@ -170,7 +170,7 @@ public class PoolableConnection extends DelegatingConnection<Connection> } catch (final SQLException e) { try { _pool.invalidateObject(this); - } catch(final IllegalStateException ise) { + } catch (final IllegalStateException ise) { // pool is closed, so close the connection passivate(); getInnermostDelegate().close(); @@ -180,18 +180,17 @@ public class PoolableConnection extends DelegatingConnection<Connection> throw new SQLException("Cannot close connection (isClosed check failed)", e); } - /* Can't set close before this code block since the connection needs to - * be open when validation runs. Can't set close after this code block - * since by then the connection will have been returned to the pool and - * may have been borrowed by another thread. Therefore, the close flag - * is set in passivate(). + /* + * Can't set close before this code block since the connection needs to be open when validation runs. Can't set + * close after this code block since by then the connection will have been returned to the pool and may have + * been borrowed by another thread. Therefore, the close flag is set in passivate(). */ if (isUnderlyingConnectionClosed) { // Abnormal close: underlying connection closed unexpectedly, so we // must destroy this proxy try { _pool.invalidateObject(this); - } catch(final IllegalStateException e) { + } catch (final IllegalStateException e) { // pool is closed, so close the connection passivate(); getInnermostDelegate().close(); @@ -203,15 +202,15 @@ public class PoolableConnection extends DelegatingConnection<Connection> // simply need to return this proxy to the pool try { _pool.returnObject(this); - } catch(final IllegalStateException e) { + } catch (final IllegalStateException e) { // pool is closed, so close the connection passivate(); getInnermostDelegate().close(); - } catch(final SQLException e) { + } catch (final SQLException e) { throw e; - } catch(final RuntimeException e) { + } catch (final RuntimeException e) { throw e; - } catch(final Exception e) { + } catch (final Exception e) { throw new SQLException("Cannot close connection (return to pool failed)", e); } } @@ -223,14 +222,9 @@ public class PoolableConnection extends DelegatingConnection<Connection> @Override public void reallyClose() throws SQLException { if (_jmxObjectName != null) { - try { - MBEAN_SERVER.unregisterMBean(_jmxObjectName); - } catch (MBeanRegistrationException | InstanceNotFoundException e) { - // Ignore - } + _jmxObjectName.unregisterMBean(); } - if (validationPreparedStatement != null) { try { validationPreparedStatement.close(); @@ -242,10 +236,8 @@ public class PoolableConnection extends DelegatingConnection<Connection> super.closeInternal(); } - /** - * Expose the {@link #toString()} method via a bean getter so it can be read - * as a property via JMX. + * Expose the {@link #toString()} method via a bean getter so it can be read as a property via JMX. */ @Override public String getToString() { @@ -255,20 +247,20 @@ public class PoolableConnection extends DelegatingConnection<Connection> /** * Validates the connection, using the following algorithm: * <ol> - * <li>If {@code fastFailValidation} (constructor argument) is {@code true} and - * this connection has previously thrown a fatal disconnection exception, - * a {@code SQLException} is thrown. </li> - * <li>If {@code sql} is null, the driver's - * #{@link Connection#isValid(int) isValid(timeout)} is called. - * If it returns {@code false}, {@code SQLException} is thrown; - * otherwise, this method returns successfully.</li> - * <li>If {@code sql} is not null, it is executed as a query and if the resulting - * {@code ResultSet} contains at least one row, this method returns - * successfully. If not, {@code SQLException} is thrown.</li> + * <li>If {@code fastFailValidation} (constructor argument) is {@code true} and this connection has previously + * thrown a fatal disconnection exception, a {@code SQLException} is thrown.</li> + * <li>If {@code sql} is null, the driver's #{@link Connection#isValid(int) isValid(timeout)} is called. If it + * returns {@code false}, {@code SQLException} is thrown; otherwise, this method returns successfully.</li> + * <li>If {@code sql} is not null, it is executed as a query and if the resulting {@code ResultSet} contains at + * least one row, this method returns successfully. If not, {@code SQLException} is thrown.</li> * </ol> - * @param sql validation query - * @param timeout validation timeout - * @throws SQLException if validation fails or an SQLException occurs during validation + * + * @param sql + * validation query + * @param timeout + * validation timeout + * @throws SQLException + * if validation fails or an SQLException occurs during validation */ public void validate(final String sql, int timeout) throws SQLException { if (_fastFailValidation && _fatalSqlExceptionThrown) { @@ -289,8 +281,7 @@ public class PoolableConnection extends DelegatingConnection<Connection> lastValidationSql = sql; // Has to be the innermost delegate else the prepared statement will // be closed when the pooled connection is passivated. - validationPreparedStatement = - getInnermostDelegateInternal().prepareStatement(sql); + validationPreparedStatement = getInnermostDelegateInternal().prepareStatement(sql); } if (timeout > 0) { @@ -298,7 +289,7 @@ public class PoolableConnection extends DelegatingConnection<Connection> } try (ResultSet rs = validationPreparedStatement.executeQuery()) { - if(!rs.next()) { + if (!rs.next()) { throw new SQLException("validationQuery didn't return a row"); } } catch (final SQLException sqle) { @@ -309,21 +300,24 @@ public class PoolableConnection extends DelegatingConnection<Connection> /** * Checks the SQLState of the input exception and any nested SQLExceptions it wraps. * <p> - * If {@link #getDisconnectSqlCodes() disconnectSQLCodes} has been set, sql states - * are compared to those in the configured list of fatal exception codes. If this - * property is not set, codes are compared against the default codes in - * #{@link Utils.DISCONNECTION_SQL_CODES} and in this case anything starting with - * #{link Utils.DISCONNECTION_SQL_CODE_PREFIX} is considered a disconnection.</p> + * If {@link #getDisconnectSqlCodes() disconnectSQLCodes} has been set, sql states are compared to those in the + * configured list of fatal exception codes. If this property is not set, codes are compared against the default + * codes in #{@link Utils.DISCONNECTION_SQL_CODES} and in this case anything starting with #{link + * Utils.DISCONNECTION_SQL_CODE_PREFIX} is considered a disconnection. + * </p> * - * @param e SQLException to be examined + * @param e + * SQLException to be examined * @return true if the exception signals a disconnection */ private boolean isDisconnectionSqlException(final SQLException e) { boolean fatalException = false; final String sqlState = e.getSQLState(); if (sqlState != null) { - fatalException = _disconnectionSqlCodes == null ? sqlState.startsWith(Utils.DISCONNECTION_SQL_CODE_PREFIX) - || Utils.DISCONNECTION_SQL_CODES.contains(sqlState) : _disconnectionSqlCodes.contains(sqlState); + fatalException = _disconnectionSqlCodes == null + ? sqlState.startsWith(Utils.DISCONNECTION_SQL_CODE_PREFIX) + || Utils.DISCONNECTION_SQL_CODES.contains(sqlState) + : _disconnectionSqlCodes.contains(sqlState); if (!fatalException) { final SQLException nextException = e.getNextException(); if (nextException != null && nextException != e) { @@ -340,4 +334,3 @@ public class PoolableConnection extends DelegatingConnection<Connection> super.handleException(e); } } -
