Connection leak when DriverDatabaseConnectionManager#createConnections throw exception

[gxgmy521]

Bug Report

For English only, other languages will not accept.

Before report a bug, make sure you have:

• Searched open and closed GitHub issues.
• Read documentation: ShardingSphere Doc.

Please pay attention on issues you submitted, because we maybe need more details.
If no response anymore and we cannot reproduce it on current information, we will close it.

Please answer these questions before submitting your issue. Thanks!

Which version of ShardingSphere did you use?

5.5.0

Which project did you use? ShardingSphere-JDBC or ShardingSphere-Proxy?

ShardingSphere-JDBC

Expected behavior

Actual behavior

Reason analyze (If you can)

In line 342, step 1, a database connection is obtained from the database connection pool.
In line 343, step 2, the previously recorded method is called for method replay.
However, if an exception was throwed from step2（for example the database server crashed）, there is no opportunity to call the connection’s close method, leading to a connection leak.

Steps to reproduce the behavior, such as: SQL to execute, sharding rule configuration, when exception occur etc.

Example codes for reproduce this issue (such as a github link).

[linghengqian]

for example the database server crashed

• This doesn't sound like there is any way to check it. java.sql.Connection does not have an interface for health checking. So I'm assuming you want to do this? Are you going to submit a PR with unit tests?

if (1 == connectionSize) {
            Connection connection = createConnection(databaseName, dataSourceName, dataSource, connectionContext.getTransactionContext());
            try {
                methodInvocationRecorder.replay(connection);
            } catch (final SQLException ex) {
                connection.close();
                throw ex;
            }
            return Collections.singletonList(connection);
        }

[gxgmy521]

for example the database server crashed

• This doesn't sound like there is any way to check it. java.sql.Connection does not have an interface for health checking. So I'm assuming you want to do this? Are you going to submit a PR with unit tests?

if (1 == connectionSize) {
            Connection connection = createConnection(databaseName, dataSourceName, dataSource, connectionContext.getTransactionContext());
            try {
                methodInvocationRecorder.replay(connection);
            } catch (final SQLException ex) {
                connection.close();
                throw ex;
            }
            return Collections.singletonList(connection);
        }

yes 。we should catch the SQLException and close the connection。

[linghengqian]

Are you going to submit a PR? The failing unit tests are clearly not in someone else's hands.

[linghengqian]

But then again, the unit tests I wrote didn't have connection leaks, there was always extra logic to close all connections. You need to provide independent unit tests.

[gxgmy521]

But then again, the unit tests I wrote didn't have connection leaks, there was always extra logic to close all connections. You need to provide independent unit tests.

Sorry I do not know how to mock the connection leaks in the unit tests 。
You mention that "there was always extra logic to close all connections." I guess you are talking about the capabilities of the connection pool (ie, the “removeAbandoned” parameter in Tomcat JDBC pool )？In my opinion ，we should close the connection immediately instead of handing it over to the connection pool。

[linghengqian]

Why do you need a mock connection?🤔 In unit tests, you can always create a connection from an h2database in mysql-compatible mode.

[gxgmy521]

Why do you need a mock connection?🤔 In unit tests, you can always create a connection from an h2database in mysql-compatible mode.

Without a mocked connection， I do not know how to control it throw an SQLException. But if mock a connection, I do not know how to re produce the connection leaks in unit test. I only reproduce the connection leak in the test environment with the help of ChaosBlade Tool.

[linghengqian]

It sounds like what you need is Mockito's @org.mockito.Spy to wrap the H2database connection?

[gxgmy521]

It sounds like what you need is Mockito's @org.mockito.Spy to wrap the H2database connection?

I do not know how to make a pr. I write unit test to produce the connection leak issue。 gxgmy521@95c4a98

[linghengqian]

I'm not sure why you don't know how to open a pull request. If you don't mind, I'd like to add a volunteer tag to the current issue? The unit tests I saw have obvious formatting issues.

[gxgmy521]

I'm not sure why you don't know how to open a pull request. If you don't mind, I'd like to add a volunteer tag to the current issue? The unit tests I saw have obvious formatting issues.

Mainly because that I am not very familiar with the code conduct and style. I will make a PR and adjust the unit test based on the https://shardingsphere.apache.org/community/en/involved/conduct/code/