Skip to content

Commit

Permalink
HHH-7020 - Connection leak with nested sessions
Browse files Browse the repository at this point in the history
  • Loading branch information
@sebersole
sebersole committed Mar 20, 2012
1 parent 5a1d523 commit 5d6d9b8
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 100 deletions.
33 changes: 21 additions & 12 deletions ...rnate-core/src/main/java/org/hibernate/engine/jdbc/internal/JdbcResourceRegistryImpl.java
View file
Expand Up @@ -26,7 +26,6 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ConcurrentModificationException;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
Expand Down Expand Up @@ -61,6 +60,7 @@ public JdbcResourceRegistryImpl(SqlExceptionHelper exceptionHelper) {
this.exceptionHelper = exceptionHelper;
}

@Override
public void register(Statement statement) {
LOG.tracev( "Registering statement [{0}]", statement );
if ( xref.containsKey( statement ) ) {
Expand All @@ -69,6 +69,7 @@ public void register(Statement statement) {
xref.put( statement, null );
}

@Override
@SuppressWarnings({ "unchecked" })
public void registerLastQuery(Statement statement) {
LOG.tracev( "Registering last query statement [{0}]", statement );
Expand All @@ -80,6 +81,7 @@ public void registerLastQuery(Statement statement) {
lastQuery = statement;
}

@Override
public void cancelLastQuery() {
try {
if (lastQuery != null) {
Expand All @@ -97,6 +99,7 @@ public void cancelLastQuery() {
}
}

@Override
public void release(Statement statement) {
LOG.tracev( "Releasing statement [{0}]", statement );
Set<ResultSet> resultSets = xref.get( statement );
Expand All @@ -110,6 +113,7 @@ public void release(Statement statement) {
close( statement );
}

@Override
public void register(ResultSet resultSet) {
LOG.tracev( "Registering result set [{0}]", resultSet );
Statement statement;
Expand All @@ -135,6 +139,7 @@ public void register(ResultSet resultSet) {
}
}

@Override
public void release(ResultSet resultSet) {
LOG.tracev( "Releasing result set [{0}]", resultSet );
Statement statement;
Expand All @@ -158,15 +163,19 @@ public void release(ResultSet resultSet) {
}
else {
boolean removed = unassociatedResultSets.remove( resultSet );
if (!removed) LOG.unregisteredResultSetWithoutStatement();
if ( !removed ) {
LOG.unregisteredResultSetWithoutStatement();
}
}
close( resultSet );
}

@Override
public boolean hasRegisteredResources() {
return ! xref.isEmpty() || ! unassociatedResultSets.isEmpty();
}

@Override
public void releaseResources() {
LOG.tracev( "Releasing JDBC container resources [{0}]", this );
cleanup();
Expand All @@ -191,6 +200,7 @@ protected void closeAll(Set<ResultSet> resultSets) {
resultSets.clear();
}

@Override
public void close() {
LOG.tracev( "Closing JDBC container [{0}]", this );
cleanup();
Expand Down Expand Up @@ -230,10 +240,12 @@ protected void close(Statement statement) {
lastQuery = null;
}
}
catch( SQLException sqle ) {
if ( LOG.isDebugEnabled() ) {
LOG.debugf( "Unable to release statement [%s]", sqle.getMessage() );
}
catch( SQLException e ) {
LOG.debugf( "Unable to release JDBC statement [%s]", e.getMessage() );
}
catch ( Exception e ) {
// try to handle general errors more elegantly
LOG.debugf( "Unable to release JDBC statement [%s]", e.getMessage() );
}
}

Expand All @@ -252,14 +264,11 @@ protected void close(ResultSet resultSet) {
resultSet.close();
}
catch( SQLException e ) {
if ( LOG.isDebugEnabled() ) {
LOG.debugf( "Unable to release result set [%s]", e.getMessage() );
}
LOG.debugf( "Unable to release JDBC result set [%s]", e.getMessage() );
}
catch ( Exception e ) {
// sybase driver (jConnect) throwing NPE here in certain cases, but we'll just handle the
// general "unexpected" case
LOG.debugf( "Could not close a JDBC result set [%s]", e.getMessage() );
// try to handle general errors more elegantly
LOG.debugf( "Unable to release JDBC result set [%s]", e.getMessage() );
}
}
}
27 changes: 22 additions & 5 deletions hibernate-core/src/main/java/org/hibernate/engine/jdbc/spi/JdbcResourceRegistry.java
View file
Expand Up @@ -28,7 +28,16 @@
import java.sql.Statement;

/**
* Defines a registry of JDBC resources related to a particular unit of work.
* Defines a registry of JDBC resources related to a particular unit of work. The main function of a
* JdbcResourceRegistry is to make sure resources get cleaned up. This is accomplished by registering all
* JDBC-related resources via the {@link #register(java.sql.Statement)} and {@link #register(java.sql.ResultSet)}
* methods. When done with these resources, they should be released by the corollary
* {@link #release(java.sql.Statement)} and {@link #release(java.sql.ResultSet)} methods. Any un-released resources
* will be released automatically when this registry is closed via {@link #close()}. Additionally,
* all registered resources can be released at any time using {@link #releaseResources()}.
* <p/>
* Additionally, a query can be registered as being able to be cancelled via the {@link #registerLastQuery}
* method. Such statements can then be cancelled by calling {@link #cancelLastQuery()}
*
* @author Steve Ebersole
*/
Expand All @@ -39,10 +48,6 @@ public interface JdbcResourceRegistry extends Serializable {
* @param statement The statement to register.
*/
public void register(Statement statement);

public void registerLastQuery(Statement statement);

public void cancelLastQuery();

/**
* Release a previously registered statement.
Expand Down Expand Up @@ -83,4 +88,16 @@ public interface JdbcResourceRegistry extends Serializable {
* After execution, the registry is considered unusable.
*/
public void close();

/**
* Register a query statement as being able to be cancelled.
*
* @param statement The cancel-able query statement.
*/
public void registerLastQuery(Statement statement);

/**
* Cancel the last query registered via {@link #registerLastQuery}
*/
public void cancelLastQuery();
}
15 changes: 7 additions & 8 deletions hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java
View file
Expand Up @@ -2193,12 +2193,8 @@ public SharedSessionBuilder interceptor() {

@Override
public SharedSessionBuilder connection() {
return connection(
session.transactionCoordinator
.getJdbcCoordinator()
.getLogicalConnection()
.getDistinctConnectionProxy()
);
this.shareTransactionContext = true;
return this;
}

@Override
Expand All @@ -2221,10 +2217,13 @@ public SharedSessionBuilder flushBeforeCompletion() {
return flushBeforeCompletion( session.flushBeforeCompletionEnabled );
}

/**
* @deprecated Use {@link #connection()} instead
*/
@Override
@Deprecated
public SharedSessionBuilder transactionContext() {
this.shareTransactionContext = true;
return this;
return connection();
}

@Override
Expand Down
75 changes: 0 additions & 75 deletions ...rnate-core/src/test/java/org/hibernate/sharedSession/SessionWithSharedConnectionTest.java
View file
Expand Up @@ -44,81 +44,6 @@
* @author Steve Ebersole
*/
public class SessionWithSharedConnectionTest extends BaseCoreFunctionalTestCase {
@Test
@TestForIssue( jiraKey = "HHH-7020" )
@FailureExpected( jiraKey = "HHH-7020" )
public void testSharedConnectionSessionClosing() {
Session session = sessionFactory().openSession();
session.getTransaction().begin();

Session secondSession = session.sessionWithOptions()
.connection()
.openSession();
secondSession.createCriteria( IrrelevantEntity.class ).list();

//the list should have registered and then released a JDBC resource
assertFalse(
((SessionImplementor) secondSession).getTransactionCoordinator()
.getJdbcCoordinator()
.getLogicalConnection()
.getResourceRegistry()
.hasRegisteredResources()
);

//there should be no registered JDBC resources on the original session
// not sure this is ultimately a valid assertion
// assertFalse(
// ((SessionImplementor) session).getTransactionCoordinator()
// .getJdbcCoordinator()
// .getLogicalConnection()
// .getResourceRegistry()
// .hasRegisteredResources()
// );

secondSession.close();

session.getTransaction().commit();
//the session should be allowed to close properly as well
session.close();
}

@Test
@TestForIssue( jiraKey = "HHH-7020" )
@FailureExpected( jiraKey = "HHH-7020" )
public void testSharedConnectionAutoClosing() {
Session session = sessionFactory().openSession();
session.getTransaction().begin();

Session secondSession = session.sessionWithOptions()
.connection()
.autoClose( true )
.openSession();
secondSession.createCriteria( IrrelevantEntity.class ).list();

//the list should have registered and then released a JDBC resource
assertFalse(
((SessionImplementor) secondSession).getTransactionCoordinator()
.getJdbcCoordinator()
.getLogicalConnection()
.getResourceRegistry()
.hasRegisteredResources()
);
//there should be no registered JDBC resources on the original session
// not sure this is ultimately a valid assertion
// assertFalse(
// ((SessionImplementor) session).getTransactionCoordinator()
// .getJdbcCoordinator()
// .getLogicalConnection()
// .getResourceRegistry()
// .hasRegisteredResources()
// );

session.getTransaction().commit();

assertTrue( ((SessionImplementor) session).isClosed() );
assertTrue( ((SessionImplementor) secondSession).isClosed() );
}

@Test
@TestForIssue( jiraKey = "HHH-7090" )
public void testSharedTransactionContextSessionClosing() {
Expand Down

0 comments on commit 5d6d9b8

Please sign in to comment.