When attempting to connect to a DataSource that specifies a database that doesn't exist, Tomcat 7.0.96 throws a NullPointerException with no useful information. See below for stack trace. Calling code is simply: try (Connection conn = dataSource.getConnection()) { ... } In this same situation, Tomcat 8.5.46 throws an informative SQLException; for example: java.sql.SQLException: Cannot create PoolableConnectionFactory (FATAL: database "labkey19.3" does not exist) An exception is appropriate, but the 8.5.46/DBCP2 behavior is obviously preferable, since it gives administrators a better understanding of what's gone wrong. Reported to the user mailing list and was encouraged to file this bug. Thanks, Adam Tomcat 7.0.96 java.lang.NullPointerException at org.apache.tomcat.dbcp.dbcp.PoolableConnectionFactory.destroyObject(PoolableConnectionFactory.java:643) at org.apache.tomcat.dbcp.dbcp.BasicDataSource.validateConnectionFactory(BasicDataSource.java:1738) at org.apache.tomcat.dbcp.dbcp.BasicDataSource.createPoolableConnectionFactory(BasicDataSource.java:1721) at org.apache.tomcat.dbcp.dbcp.BasicDataSource.createDataSource(BasicDataSource.java:1486) at org.apache.tomcat.dbcp.dbcp.BasicDataSource.getConnection(BasicDataSource.java:1103) at org.labkey.api.data.DbScope.<init>(DbScope.java:337) at org.labkey.api.data.DbScope.addScope(DbScope.java:1275) at org.labkey.api.data.DbScope.initializeScopes(DbScope.java:1243) at org.labkey.api.module.ModuleLoader.initializeDataSources(ModuleLoader.java:1004) at org.labkey.api.module.ModuleLoader.doInit(ModuleLoader.java:352) at org.labkey.api.module.ModuleLoader.init(ModuleLoader.java:249) at org.apache.catalina.core.ApplicationFilterConfig.initFilter(ApplicationFilterConfig.java:285) at org.apache.catalina.core.ApplicationFilterConfig.getFilter(ApplicationFilterConfig.java:266) at org.apache.catalina.core.ApplicationFilterConfig.<init>(ApplicationFilterConfig.java:108) at org.apache.catalina.core.StandardContext.filterStart(StandardContext.java:5037) at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5739) at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:145) at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:1018) at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:994) at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:662) at org.apache.catalina.startup.HostConfig.deployDescriptor(HostConfig.java:712) at org.apache.catalina.startup.HostConfig$DeployDescriptor.run(HostConfig.java:2002) at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) at java.base/java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:264) at java.base/java.util.concurrent.FutureTask.run(FutureTask.java) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:830) Tomcat 8.5.46 java.sql.SQLException: Cannot create PoolableConnectionFactory (FATAL: database "labkey19.3" does not exist) at org.apache.tomcat.dbcp.dbcp2.BasicDataSource.createPoolableConnectionFactory(BasicDataSource.java:669) at org.apache.tomcat.dbcp.dbcp2.BasicDataSource.createDataSource(BasicDataSource.java:544) at org.apache.tomcat.dbcp.dbcp2.BasicDataSource.getConnection(BasicDataSource.java:753) at org.labkey.api.data.DbScope.<init>(DbScope.java:337) at org.labkey.api.data.DbScope.addScope(DbScope.java:1275) at org.labkey.api.data.DbScope.initializeScopes(DbScope.java:1243) at org.labkey.api.module.ModuleLoader.initializeDataSources(ModuleLoader.java:1004) at org.labkey.api.module.ModuleLoader.doInit(ModuleLoader.java:352) at org.labkey.api.module.ModuleLoader.init(ModuleLoader.java:249) at org.apache.catalina.core.ApplicationFilterConfig.initFilter(ApplicationFilterConfig.java:283) at org.apache.catalina.core.ApplicationFilterConfig.getFilter(ApplicationFilterConfig.java:264) at org.apache.catalina.core.ApplicationFilterConfig.<init>(ApplicationFilterConfig.java:108) at org.apache.catalina.core.StandardContext.filterStart(StandardContext.java:4546) at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5191) at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150) at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:743) at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:719) at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:714) at org.apache.catalina.startup.HostConfig.deployDescriptor(HostConfig.java:614) at org.apache.catalina.startup.HostConfig$DeployDescriptor.run(HostConfig.java:1823) at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) at java.base/java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:264) at java.base/java.util.concurrent.FutureTask.run(FutureTask.java) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:830) Caused by: org.postgresql.util.PSQLException: FATAL: database "labkey19.3" does not exist at org.postgresql.core.v3.QueryExecutorImpl.receiveErrorResponse(QueryExecutorImpl.java:2440) at org.postgresql.core.v3.QueryExecutorImpl.readStartupMessages(QueryExecutorImpl.java:2559) at org.postgresql.core.v3.QueryExecutorImpl.<init>(QueryExecutorImpl.java:133) at org.postgresql.core.v3.ConnectionFactoryImpl.openConnectionImpl(ConnectionFactoryImpl.java:250) at org.postgresql.core.ConnectionFactory.openConnection(ConnectionFactory.java:49) at org.postgresql.jdbc.PgConnection.<init>(PgConnection.java:195) at org.postgresql.Driver.makeConnection(Driver.java:454) at org.postgresql.Driver.connect(Driver.java:256) at org.apache.tomcat.dbcp.dbcp2.DriverConnectionFactory.createConnection(DriverConnectionFactory.java:55) at org.apache.tomcat.dbcp.dbcp2.PoolableConnectionFactory.makeObject(PoolableConnectionFactory.java:355) at org.apache.tomcat.dbcp.dbcp2.BasicDataSource.validateConnectionFactory(BasicDataSource.java:115) at org.apache.tomcat.dbcp.dbcp2.BasicDataSource.createPoolableConnectionFactory(BasicDataSource.java:665) ... 25 more
Created attachment 36823 [details] A draft to solve this issue.
This is a regression from the generics conversion in PoolableConnectionFactory. The original DBCP 1.x code effectively null-checked the object to be destroyed: public void destroyObject(Object obj) throws Exception { if(obj instanceof PoolableConnection) { ((PoolableConnection)obj).reallyClose(); } Removing the instanceOf check makes NPE possible: public void destroyObject(PoolableConnection obj) throws Exception { obj.reallyClose(); } Solution is to add an explicit null check in destroyObject. Similar changes should be made to activate, passivate, validate methods.
Created attachment 36824 [details] A new draft patch for review. Hi Phil Steitz, think your for your reminding. I update my patch. The new patch add null check in method destroyObject. I don't modify method activateObject, validateConnection and passivateObject because I consider that a patch should minumize and avoid too many changes. When the reviewer agree to the change and the test method in this path, I will add another patch which changes the method activateObject, validateConnection and passivateObject. I will squash all the commit and submit a pull request which make it easy to merge. Please review the patch. Thanks.
Both reviews and edits can all be done in a PR. Why attach successive patches to this BZ issue when you plan in issuing a PR anyway?
I once submitted a PR to solve a issue. But the PR was not good enough to merge so that the committer solved the issue by himself and closed my PR without merging. As a result, I attach the patch to this BZ for committers to review and comment this time so that I can modify my patch and avoid my PR to be rejected. It is better for committer to comment, give some recommendation and let the PR submitter to modify the PR instead of doing similar thing by themselves after reviewing the PR. Maybe the situation of my previous PR happened by accident and is not the typical review process. I will choose only one way(patch or PR) to contribute in the future according to your suggestion which could avoid repeat. In this BZ, I will just use the patch file and won't use PR.
(In reply to Christopher Schultz from comment #4) It is much better to keep discussion in one place in Bugzilla, rather than scattered among maybe several PRs. I was wondering why this bug report is against Tomcat instead of the upstream Apache Commons DBCP project, by Phil Steitz's comment 2 gives an explanation.
Thanks for the patch. I applied the fix but not the unit test as Phil Steitz has back-ported the Commons DBCP 1.x unit tests that also catch this error.