You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by tu...@apache.org on 2021/04/16 07:28:07 UTC
[nifi] branch main updated: NIFI-8429 Changing DBCPConnectionPool
driver manager in order to prevent from leaking
This is an automated email from the ASF dual-hosted git repository.
turcsanyi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi.git
The following commit(s) were added to refs/heads/main by this push:
new f0ca63b NIFI-8429 Changing DBCPConnectionPool driver manager in order to prevent from leaking
f0ca63b is described below
commit f0ca63b2ab4a630f600c66cf7c581a4565bb525a
Author: Bence Simon <si...@gmail.com>
AuthorDate: Wed Apr 14 23:05:13 2021 +0200
NIFI-8429 Changing DBCPConnectionPool driver manager in order to prevent from leaking
NIFI-8429 Refining error handling based on review comment
This closes #5003.
Signed-off-by: Peter Turcsanyi <tu...@apache.org>
---
.../org/apache/nifi/dbcp/DBCPConnectionPool.java | 73 ++++++++-------------
.../main/java/org/apache/nifi/dbcp/DriverShim.java | 74 ----------------------
2 files changed, 28 insertions(+), 119 deletions(-)
diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DBCPConnectionPool.java b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DBCPConnectionPool.java
index 3972cec..b43f691 100644
--- a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DBCPConnectionPool.java
+++ b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DBCPConnectionPool.java
@@ -21,6 +21,7 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import org.apache.nifi.annotation.behavior.DynamicProperties;
import org.apache.nifi.annotation.behavior.DynamicProperty;
+import org.apache.nifi.annotation.behavior.RequiresInstanceClassLoading;
import org.apache.nifi.annotation.documentation.CapabilityDescription;
import org.apache.nifi.annotation.documentation.Tags;
import org.apache.nifi.annotation.lifecycle.OnDisabled;
@@ -43,10 +44,8 @@ import org.apache.nifi.security.krb.KerberosAction;
import org.apache.nifi.security.krb.KerberosKeytabUser;
import org.apache.nifi.security.krb.KerberosPasswordUser;
import org.apache.nifi.security.krb.KerberosUser;
-import org.apache.nifi.util.file.classloader.ClassLoaderUtils;
import javax.security.auth.login.LoginException;
-import java.net.MalformedURLException;
import java.sql.Connection;
import java.sql.Driver;
import java.sql.DriverManager;
@@ -74,6 +73,7 @@ import java.util.stream.Collectors;
expressionLanguageScope = ExpressionLanguageScope.NONE,
description = "JDBC driver property name prefixed with 'SENSITIVE.' handled as a sensitive property.")
})
+@RequiresInstanceClassLoading
public class DBCPConnectionPool extends AbstractControllerService implements DBCPService {
/** Property Name Prefix for Sensitive Dynamic Properties */
protected static final String SENSITIVE_PROPERTY_PREFIX = "SENSITIVE.";
@@ -131,6 +131,7 @@ public class DBCPConnectionPool extends AbstractControllerService implements DBC
.required(false)
.identifiesExternalResource(ResourceCardinality.MULTIPLE, ResourceType.FILE, ResourceType.DIRECTORY, ResourceType.URL)
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+ .dynamicallyModifiesClasspath(true)
.build();
public static final PropertyDescriptor DB_USER = new PropertyDescriptor.Builder()
@@ -386,9 +387,10 @@ public class DBCPConnectionPool extends AbstractControllerService implements DBC
@OnEnabled
public void onConfigured(final ConfigurationContext context) throws InitializationException {
- final String drv = context.getProperty(DB_DRIVERNAME).evaluateAttributeExpressions().getValue();
+ final String driverName = context.getProperty(DB_DRIVERNAME).evaluateAttributeExpressions().getValue();
final String user = context.getProperty(DB_USER).evaluateAttributeExpressions().getValue();
final String passw = context.getProperty(DB_PASSWORD).evaluateAttributeExpressions().getValue();
+ final String dburl = context.getProperty(DATABASE_URL).evaluateAttributeExpressions().getValue();
final Integer maxTotal = context.getProperty(MAX_TOTAL_CONNECTIONS).evaluateAttributeExpressions().asInteger();
final String validationQuery = context.getProperty(VALIDATION_QUERY).evaluateAttributeExpressions().getValue();
final Long maxWaitMillis = extractMillisWithInfinite(context.getProperty(MAX_WAIT_TIME).evaluateAttributeExpressions());
@@ -417,14 +419,7 @@ public class DBCPConnectionPool extends AbstractControllerService implements DBC
}
dataSource = new BasicDataSource();
- dataSource.setDriverClassName(drv);
-
- // Optional driver URL, when exist, this URL will be used to locate driver jar file location
- final String urlString = context.getProperty(DB_DRIVER_LOCATION).evaluateAttributeExpressions().getValue();
- dataSource.setDriverClassLoader(getDriverClassLoader(urlString, drv));
-
- final String dburl = context.getProperty(DATABASE_URL).evaluateAttributeExpressions().getValue();
-
+ dataSource.setDriver(getDriver(driverName, dburl));
dataSource.setMaxWaitMillis(maxWaitMillis);
dataSource.setMaxTotal(maxTotal);
dataSource.setMinIdle(minIdle);
@@ -460,47 +455,35 @@ public class DBCPConnectionPool extends AbstractControllerService implements DBC
});
}
- private Long extractMillisWithInfinite(PropertyValue prop) {
- return "-1".equals(prop.getValue()) ? -1 : prop.asTimePeriod(TimeUnit.MILLISECONDS);
- }
+ private Driver getDriver(final String driverName, final String url) {
+ final Class<?> clazz;
- /**
- * using Thread.currentThread().getContextClassLoader(); will ensure that you are using the ClassLoader for you NAR.
- *
- * @throws InitializationException
- * if there is a problem obtaining the ClassLoader
- */
- protected ClassLoader getDriverClassLoader(String locationString, String drvName) throws InitializationException {
- if (locationString != null && locationString.length() > 0) {
+ try {
+ clazz = Class.forName(driverName);
+ } catch (final ClassNotFoundException e) {
+ throw new ProcessException("Driver class " + driverName + " is not found", e);
+ }
+
+ try {
+ return DriverManager.getDriver(url);
+ } catch (final SQLException e) {
+ // In case the driver is not registered by the implementation, we explicitly try to register it.
try {
- // Split and trim the entries
- final ClassLoader classLoader = ClassLoaderUtils.getCustomClassLoader(
- locationString,
- this.getClass().getClassLoader(),
- (dir, name) -> name != null && name.endsWith(".jar")
- );
-
- // Workaround which allows to use URLClassLoader for JDBC driver loading.
- // (Because the DriverManager will refuse to use a driver not loaded by the system ClassLoader.)
- final Class<?> clazz = Class.forName(drvName, true, classLoader);
- if (clazz == null) {
- throw new InitializationException("Can't load Database Driver " + drvName);
- }
final Driver driver = (Driver) clazz.newInstance();
- DriverManager.registerDriver(new DriverShim(driver));
-
- return classLoader;
- } catch (final MalformedURLException e) {
- throw new InitializationException("Invalid Database Driver Jar Url", e);
- } catch (final Exception e) {
- throw new InitializationException("Can't load Database Driver", e);
+ DriverManager.registerDriver(driver);
+ return DriverManager.getDriver(url);
+ } catch (final SQLException e2) {
+ throw new ProcessException("No suitable driver for the given Database Connection URL", e2);
+ } catch (final IllegalAccessException | InstantiationException e2) {
+ throw new ProcessException("Creating driver instance is failed", e2);
}
- } else {
- // That will ensure that you are using the ClassLoader for you NAR.
- return Thread.currentThread().getContextClassLoader();
}
}
+ private Long extractMillisWithInfinite(PropertyValue prop) {
+ return "-1".equals(prop.getValue()) ? -1 : prop.asTimePeriod(TimeUnit.MILLISECONDS);
+ }
+
/**
* Shutdown pool, close all open connections.
* If a principal is authenticated with a KDC, that principal is logged out.
diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DriverShim.java b/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DriverShim.java
deleted file mode 100644
index 596f171..0000000
--- a/nifi-nar-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DriverShim.java
+++ /dev/null
@@ -1,74 +0,0 @@
-/*
- * 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.nifi.dbcp;
-
-import java.sql.Connection;
-import java.sql.Driver;
-import java.sql.DriverPropertyInfo;
-import java.sql.SQLException;
-import java.sql.SQLFeatureNotSupportedException;
-import java.util.Properties;
-import java.util.logging.Logger;
-
-/**
- * Workaround which allows to use URLClassLoader for JDBC driver loading.
- * (Because the DriverManager will refuse to use a driver not loaded by the system ClassLoader.)
- *
- */
-class DriverShim implements Driver {
- private Driver driver;
-
- DriverShim(Driver d) {
- this.driver = d;
- }
-
- @Override
- public boolean acceptsURL(String u) throws SQLException {
- return this.driver.acceptsURL(u);
- }
-
- @Override
- public Connection connect(String u, Properties p) throws SQLException {
- return this.driver.connect(u, p);
- }
-
- @Override
- public int getMajorVersion() {
- return this.driver.getMajorVersion();
- }
-
- @Override
- public int getMinorVersion() {
- return this.driver.getMinorVersion();
- }
-
- @Override
- public DriverPropertyInfo[] getPropertyInfo(String u, Properties p) throws SQLException {
- return this.driver.getPropertyInfo(u, p);
- }
-
- @Override
- public boolean jdbcCompliant() {
- return this.driver.jdbcCompliant();
- }
-
- @Override
- public Logger getParentLogger() throws SQLFeatureNotSupportedException {
- return driver.getParentLogger();
- }
-
-}
\ No newline at end of file