Skip to content

Commit 668815c

Browse files
committed
Close connection pool if driver creation fails
Connection pool (`SocketConnectionPool`) is created before drivers and was never closed if driver creation failed. This was especially possible with `RoutingDriver` which tries to build routing table in constructor. This commit adds closing of the connection pool when creation of driver fails. It also extracts driver creation logic into an internal class `DriverFactory` to make it testable.
1 parent 4b0c5aa commit 668815c

File tree

3 files changed

+327
-126
lines changed

3 files changed

+327
-126
lines changed
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
/*
2+
* Copyright (c) 2002-2016 "Neo Technology,"
3+
* Network Engine for Objects in Lund AB [http://neotechnology.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
package org.neo4j.driver.internal;
20+
21+
import java.io.IOException;
22+
import java.net.URI;
23+
import java.security.GeneralSecurityException;
24+
25+
import org.neo4j.driver.internal.cluster.RoutingSettings;
26+
import org.neo4j.driver.internal.net.BoltServerAddress;
27+
import org.neo4j.driver.internal.net.SocketConnector;
28+
import org.neo4j.driver.internal.net.pooling.PoolSettings;
29+
import org.neo4j.driver.internal.net.pooling.SocketConnectionPool;
30+
import org.neo4j.driver.internal.security.SecurityPlan;
31+
import org.neo4j.driver.internal.spi.ConnectionPool;
32+
import org.neo4j.driver.internal.spi.Connector;
33+
import org.neo4j.driver.internal.util.Clock;
34+
import org.neo4j.driver.v1.AuthToken;
35+
import org.neo4j.driver.v1.AuthTokens;
36+
import org.neo4j.driver.v1.Config;
37+
import org.neo4j.driver.v1.Driver;
38+
import org.neo4j.driver.v1.Logger;
39+
import org.neo4j.driver.v1.exceptions.ClientException;
40+
41+
import static java.lang.String.format;
42+
import static org.neo4j.driver.internal.security.SecurityPlan.insecure;
43+
import static org.neo4j.driver.v1.Config.EncryptionLevel.REQUIRED;
44+
45+
public class DriverFactory
46+
{
47+
public final Driver newInstance( URI uri, AuthToken authToken, RoutingSettings routingSettings, Config config )
48+
{
49+
BoltServerAddress address = BoltServerAddress.from( uri );
50+
SecurityPlan securityPlan = createSecurityPlan( address, config );
51+
ConnectionPool connectionPool = createConnectionPool( authToken, securityPlan, config );
52+
53+
try
54+
{
55+
return createDriver( address, uri.getScheme(), connectionPool, config, routingSettings, securityPlan );
56+
}
57+
catch ( Throwable driverError )
58+
{
59+
// we need to close the connection pool if driver creation threw exception
60+
try
61+
{
62+
connectionPool.close();
63+
}
64+
catch ( Throwable closeError )
65+
{
66+
driverError.addSuppressed( closeError );
67+
}
68+
throw driverError;
69+
}
70+
}
71+
72+
private Driver createDriver( BoltServerAddress address, String scheme, ConnectionPool connectionPool,
73+
Config config, RoutingSettings routingSettings, SecurityPlan securityPlan )
74+
{
75+
switch ( scheme.toLowerCase() )
76+
{
77+
case "bolt":
78+
return createDirectDriver( address, connectionPool, config, securityPlan );
79+
case "bolt+routing":
80+
return createRoutingDriver( address, connectionPool, config, routingSettings, securityPlan );
81+
default:
82+
throw new ClientException( format( "Unsupported URI scheme: %s", scheme ) );
83+
}
84+
}
85+
86+
/**
87+
* Creates new {@link DirectDriver}.
88+
* <p>
89+
* <b>This method is package-private only for testing</b>
90+
*/
91+
DirectDriver createDirectDriver( BoltServerAddress address, ConnectionPool connectionPool, Config config,
92+
SecurityPlan securityPlan )
93+
{
94+
return new DirectDriver( address, connectionPool, securityPlan, config.logging() );
95+
}
96+
97+
/**
98+
* Creates new {@link RoutingDriver}.
99+
* <p>
100+
* <b>This method is package-private only for testing</b>
101+
*/
102+
RoutingDriver createRoutingDriver( BoltServerAddress address, ConnectionPool connectionPool,
103+
Config config, RoutingSettings routingSettings, SecurityPlan securityPlan )
104+
{
105+
return new RoutingDriver( routingSettings, address, connectionPool, securityPlan, Clock.SYSTEM,
106+
config.logging() );
107+
}
108+
109+
/**
110+
* Creates new {@link ConnectionPool}.
111+
* <p>
112+
* <b>This method is package-private only for testing</b>
113+
*/
114+
ConnectionPool createConnectionPool( AuthToken authToken, SecurityPlan securityPlan, Config config )
115+
{
116+
authToken = authToken == null ? AuthTokens.none() : authToken;
117+
118+
ConnectionSettings connectionSettings = new ConnectionSettings( authToken );
119+
PoolSettings poolSettings = new PoolSettings( config.maxIdleConnectionPoolSize() );
120+
Connector connector = new SocketConnector( connectionSettings, securityPlan, config.logging() );
121+
122+
return new SocketConnectionPool( poolSettings, connector, Clock.SYSTEM, config.logging() );
123+
}
124+
125+
private static SecurityPlan createSecurityPlan( BoltServerAddress address, Config config )
126+
{
127+
try
128+
{
129+
return createSecurityPlanImpl( address, config );
130+
}
131+
catch ( GeneralSecurityException | IOException ex )
132+
{
133+
throw new ClientException( "Unable to establish SSL parameters", ex );
134+
}
135+
}
136+
137+
/*
138+
* Establish a complete SecurityPlan based on the details provided for
139+
* driver construction.
140+
*/
141+
private static SecurityPlan createSecurityPlanImpl( BoltServerAddress address, Config config )
142+
throws GeneralSecurityException, IOException
143+
{
144+
Config.EncryptionLevel encryptionLevel = config.encryptionLevel();
145+
boolean requiresEncryption = encryptionLevel.equals( REQUIRED );
146+
147+
if ( requiresEncryption )
148+
{
149+
Logger logger = config.logging().getLog( "session" );
150+
switch ( config.trustStrategy().strategy() )
151+
{
152+
153+
// DEPRECATED CASES //
154+
case TRUST_ON_FIRST_USE:
155+
logger.warn(
156+
"Option `TRUST_ON_FIRST_USE` has been deprecated and will be removed in a future " +
157+
"version of the driver. Please switch to use `TRUST_ALL_CERTIFICATES` instead." );
158+
return SecurityPlan.forTrustOnFirstUse( config.trustStrategy().certFile(), address, logger );
159+
case TRUST_SIGNED_CERTIFICATES:
160+
logger.warn(
161+
"Option `TRUST_SIGNED_CERTIFICATE` has been deprecated and will be removed in a future " +
162+
"version of the driver. Please switch to use `TRUST_CUSTOM_CA_SIGNED_CERTIFICATES` instead." );
163+
// intentional fallthrough
164+
// END OF DEPRECATED CASES //
165+
166+
case TRUST_CUSTOM_CA_SIGNED_CERTIFICATES:
167+
return SecurityPlan.forCustomCASignedCertificates( config.trustStrategy().certFile() );
168+
case TRUST_SYSTEM_CA_SIGNED_CERTIFICATES:
169+
return SecurityPlan.forSystemCASignedCertificates();
170+
case TRUST_ALL_CERTIFICATES:
171+
return SecurityPlan.forAllCertificates();
172+
default:
173+
throw new ClientException(
174+
"Unknown TLS authentication strategy: " + config.trustStrategy().strategy().name() );
175+
}
176+
}
177+
else
178+
{
179+
return insecure();
180+
}
181+
}
182+
}

driver/src/main/java/org/neo4j/driver/v1/GraphDatabase.java

Lines changed: 3 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -18,29 +18,9 @@
1818
*/
1919
package org.neo4j.driver.v1;
2020

21-
import java.io.IOException;
2221
import java.net.URI;
23-
import java.security.GeneralSecurityException;
2422

25-
import org.neo4j.driver.internal.ConnectionSettings;
26-
import org.neo4j.driver.internal.DirectDriver;
27-
import org.neo4j.driver.internal.NetworkSession;
28-
import org.neo4j.driver.internal.RoutingDriver;
29-
import org.neo4j.driver.internal.net.BoltServerAddress;
30-
import org.neo4j.driver.internal.net.SocketConnector;
31-
import org.neo4j.driver.internal.net.pooling.PoolSettings;
32-
import org.neo4j.driver.internal.net.pooling.SocketConnectionPool;
33-
import org.neo4j.driver.internal.security.SecurityPlan;
34-
import org.neo4j.driver.internal.spi.Connection;
35-
import org.neo4j.driver.internal.spi.ConnectionPool;
36-
import org.neo4j.driver.internal.spi.Connector;
37-
import org.neo4j.driver.internal.util.Clock;
38-
import org.neo4j.driver.v1.exceptions.ClientException;
39-
import org.neo4j.driver.v1.util.Function;
40-
41-
import static java.lang.String.format;
42-
import static org.neo4j.driver.internal.security.SecurityPlan.insecure;
43-
import static org.neo4j.driver.v1.Config.EncryptionLevel.REQUIRED;
23+
import org.neo4j.driver.internal.DriverFactory;
4424

4525
/**
4626
* Creates {@link Driver drivers}, optionally letting you {@link #driver(URI, Config)} to configure them.
@@ -49,17 +29,6 @@
4929
*/
5030
public class GraphDatabase
5131
{
52-
53-
private static final Function<Connection,Session>
54-
SESSION_PROVIDER = new Function<Connection,Session>()
55-
{
56-
@Override
57-
public Session apply( Connection connection )
58-
{
59-
return new NetworkSession( connection );
60-
}
61-
};
62-
6332
/**
6433
* Return a driver for a Neo4j instance with the default configuration settings
6534
*
@@ -153,101 +122,9 @@ public static Driver driver( String uri, AuthToken authToken, Config config )
153122
*/
154123
public static Driver driver( URI uri, AuthToken authToken, Config config )
155124
{
156-
// Break down the URI into its constituent parts
157-
String scheme = uri.getScheme();
158-
BoltServerAddress address = BoltServerAddress.from( uri );
159-
160125
// Make sure we have some configuration to play with
161-
if ( config == null )
162-
{
163-
config = Config.defaultConfig();
164-
}
165-
166-
// Construct security plan
167-
SecurityPlan securityPlan;
168-
try
169-
{
170-
securityPlan = createSecurityPlan( address, config );
171-
}
172-
catch ( GeneralSecurityException | IOException ex )
173-
{
174-
throw new ClientException( "Unable to establish SSL parameters", ex );
175-
}
176-
177-
ConnectionPool connectionPool = createConnectionPool( authToken, securityPlan, config );
178-
179-
switch ( scheme.toLowerCase() )
180-
{
181-
case "bolt":
182-
return new DirectDriver( address, connectionPool, securityPlan, config.logging() );
183-
case "bolt+routing":
184-
return new RoutingDriver(
185-
config.routingSettings(),
186-
address,
187-
connectionPool,
188-
securityPlan,
189-
Clock.SYSTEM,
190-
config.logging() );
191-
default:
192-
throw new ClientException( format( "Unsupported URI scheme: %s", scheme ) );
193-
}
194-
}
195-
196-
private static ConnectionPool createConnectionPool( AuthToken authToken, SecurityPlan securityPlan,
197-
Config config )
198-
{
199-
authToken = authToken == null ? AuthTokens.none() : authToken;
200-
201-
ConnectionSettings connectionSettings = new ConnectionSettings( authToken );
202-
PoolSettings poolSettings = new PoolSettings( config.maxIdleConnectionPoolSize() );
203-
Connector connector = new SocketConnector( connectionSettings, securityPlan, config.logging() );
204-
205-
return new SocketConnectionPool( poolSettings, connector, Clock.SYSTEM, config.logging() );
206-
}
207-
208-
/*
209-
* Establish a complete SecurityPlan based on the details provided for
210-
* driver construction.
211-
*/
212-
private static SecurityPlan createSecurityPlan( BoltServerAddress address, Config config )
213-
throws GeneralSecurityException, IOException
214-
{
215-
Config.EncryptionLevel encryptionLevel = config.encryptionLevel();
216-
boolean requiresEncryption = encryptionLevel.equals( REQUIRED );
217-
218-
if ( requiresEncryption )
219-
{
220-
Logger logger = config.logging().getLog( "session" );
221-
switch ( config.trustStrategy().strategy() )
222-
{
223-
224-
// DEPRECATED CASES //
225-
case TRUST_ON_FIRST_USE:
226-
logger.warn(
227-
"Option `TRUST_ON_FIRST_USE` has been deprecated and will be removed in a future " +
228-
"version of the driver. Please switch to use `TRUST_ALL_CERTIFICATES` instead." );
229-
return SecurityPlan.forTrustOnFirstUse( config.trustStrategy().certFile(), address, logger );
230-
case TRUST_SIGNED_CERTIFICATES:
231-
logger.warn(
232-
"Option `TRUST_SIGNED_CERTIFICATE` has been deprecated and will be removed in a future " +
233-
"version of the driver. Please switch to use `TRUST_CUSTOM_CA_SIGNED_CERTIFICATES` instead." );
234-
// intentional fallthrough
235-
// END OF DEPRECATED CASES //
126+
config = config == null ? Config.defaultConfig() : config;
236127

237-
case TRUST_CUSTOM_CA_SIGNED_CERTIFICATES:
238-
return SecurityPlan.forCustomCASignedCertificates( config.trustStrategy().certFile() );
239-
case TRUST_SYSTEM_CA_SIGNED_CERTIFICATES:
240-
return SecurityPlan.forSystemCASignedCertificates();
241-
case TRUST_ALL_CERTIFICATES:
242-
return SecurityPlan.forAllCertificates();
243-
default:
244-
throw new ClientException(
245-
"Unknown TLS authentication strategy: " + config.trustStrategy().strategy().name() );
246-
}
247-
}
248-
else
249-
{
250-
return insecure();
251-
}
128+
return new DriverFactory().newInstance( uri, authToken, config.routingSettings(), config );
252129
}
253130
}

0 commit comments

Comments
 (0)