bobpaulin commented on code in PR #10693:
URL: https://github.com/apache/nifi/pull/10693#discussion_r2651849191
##########
nifi-extension-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/main/java/org/apache/nifi/dbcp/DBCPConnectionPool.java:
##########
@@ -262,14 +262,21 @@ protected Driver getDriver(final String driverName, final
String url) {
}
try {
- return DriverManager.getDriver(url);
+ final Driver driver = DriverManager.getDriver(url);
+ // Ensure drivers that register themselves during class loading
can be set as the registeredDriver.
+ // This ensures drivers that register themselves can be
deregisterd when the componet is removed.
+ // These drivers should be loaded in the same InstanceClassloader
that load this component
+ if (driver != registeredDriver
+ &&
driver.getClass().getClassLoader().equals(getClass().getClassLoader())) {
Review Comment:
The general case that this statement catches is when the registeredDriver is
not set and was loaded by the Driver during classloading as a static method
https://github.com/pgjdbc/pgjdbc/blob/e3ea9a3bfce6330aea67b3ff555cd7c32a755134/pgjdbc/src/main/java/org/postgresql/Driver.java#L76
In normal cases this is done by the InstanceClassLoader so I can see why
this check could be skipped.
I left the check in around a concern I had for a driver registered to a
parent classloader. IE someone adds the driver to NIFI_HOME/lib .
In that case if the driver was loaded in the parent I would not want the
controller service code to deregister the driver since it would be in a shared
classloader but available to the InstanceClassLoader. So probably not normal
operation but I wanted to be defensive incase users took this approach.
In the above potentially outside of normal case we'd skip the code to
deregister and set the registeredDriver reference.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
