HHH-7020 - Connection leak with nested sessions

This commit is contained in:
Steve Ebersole 2012-03-19 19:09:44 -05:00
parent 5a1d523b4d
commit 5d6d9b87c1
4 changed files with 50 additions and 100 deletions

View File

@ -26,7 +26,6 @@ package org.hibernate.engine.jdbc.internal;
import java.sql.ResultSet; import java.sql.ResultSet;
import java.sql.SQLException; import java.sql.SQLException;
import java.sql.Statement; import java.sql.Statement;
import java.util.ConcurrentModificationException;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.Map; import java.util.Map;
@ -61,6 +60,7 @@ public class JdbcResourceRegistryImpl implements JdbcResourceRegistry {
this.exceptionHelper = exceptionHelper; this.exceptionHelper = exceptionHelper;
} }
@Override
public void register(Statement statement) { public void register(Statement statement) {
LOG.tracev( "Registering statement [{0}]", statement ); LOG.tracev( "Registering statement [{0}]", statement );
if ( xref.containsKey( statement ) ) { if ( xref.containsKey( statement ) ) {
@ -69,6 +69,7 @@ public class JdbcResourceRegistryImpl implements JdbcResourceRegistry {
xref.put( statement, null ); xref.put( statement, null );
} }
@Override
@SuppressWarnings({ "unchecked" }) @SuppressWarnings({ "unchecked" })
public void registerLastQuery(Statement statement) { public void registerLastQuery(Statement statement) {
LOG.tracev( "Registering last query statement [{0}]", statement ); LOG.tracev( "Registering last query statement [{0}]", statement );
@ -80,6 +81,7 @@ public class JdbcResourceRegistryImpl implements JdbcResourceRegistry {
lastQuery = statement; lastQuery = statement;
} }
@Override
public void cancelLastQuery() { public void cancelLastQuery() {
try { try {
if (lastQuery != null) { if (lastQuery != null) {
@ -97,6 +99,7 @@ public class JdbcResourceRegistryImpl implements JdbcResourceRegistry {
} }
} }
@Override
public void release(Statement statement) { public void release(Statement statement) {
LOG.tracev( "Releasing statement [{0}]", statement ); LOG.tracev( "Releasing statement [{0}]", statement );
Set<ResultSet> resultSets = xref.get( statement ); Set<ResultSet> resultSets = xref.get( statement );
@ -110,6 +113,7 @@ public class JdbcResourceRegistryImpl implements JdbcResourceRegistry {
close( statement ); close( statement );
} }
@Override
public void register(ResultSet resultSet) { public void register(ResultSet resultSet) {
LOG.tracev( "Registering result set [{0}]", resultSet ); LOG.tracev( "Registering result set [{0}]", resultSet );
Statement statement; Statement statement;
@ -135,6 +139,7 @@ public class JdbcResourceRegistryImpl implements JdbcResourceRegistry {
} }
} }
@Override
public void release(ResultSet resultSet) { public void release(ResultSet resultSet) {
LOG.tracev( "Releasing result set [{0}]", resultSet ); LOG.tracev( "Releasing result set [{0}]", resultSet );
Statement statement; Statement statement;
@ -158,15 +163,19 @@ public class JdbcResourceRegistryImpl implements JdbcResourceRegistry {
} }
else { else {
boolean removed = unassociatedResultSets.remove( resultSet ); boolean removed = unassociatedResultSets.remove( resultSet );
if (!removed) LOG.unregisteredResultSetWithoutStatement(); if ( !removed ) {
LOG.unregisteredResultSetWithoutStatement();
}
} }
close( resultSet ); close( resultSet );
} }
@Override
public boolean hasRegisteredResources() { public boolean hasRegisteredResources() {
return ! xref.isEmpty() || ! unassociatedResultSets.isEmpty(); return ! xref.isEmpty() || ! unassociatedResultSets.isEmpty();
} }
@Override
public void releaseResources() { public void releaseResources() {
LOG.tracev( "Releasing JDBC container resources [{0}]", this ); LOG.tracev( "Releasing JDBC container resources [{0}]", this );
cleanup(); cleanup();
@ -191,6 +200,7 @@ public class JdbcResourceRegistryImpl implements JdbcResourceRegistry {
resultSets.clear(); resultSets.clear();
} }
@Override
public void close() { public void close() {
LOG.tracev( "Closing JDBC container [{0}]", this ); LOG.tracev( "Closing JDBC container [{0}]", this );
cleanup(); cleanup();
@ -230,10 +240,12 @@ public class JdbcResourceRegistryImpl implements JdbcResourceRegistry {
lastQuery = null; lastQuery = null;
} }
} }
catch( SQLException sqle ) { catch( SQLException e ) {
if ( LOG.isDebugEnabled() ) { LOG.debugf( "Unable to release JDBC statement [%s]", e.getMessage() );
LOG.debugf( "Unable to release statement [%s]", sqle.getMessage() );
} }
catch ( Exception e ) {
// try to handle general errors more elegantly
LOG.debugf( "Unable to release JDBC statement [%s]", e.getMessage() );
} }
} }
@ -252,14 +264,11 @@ public class JdbcResourceRegistryImpl implements JdbcResourceRegistry {
resultSet.close(); resultSet.close();
} }
catch( SQLException e ) { catch( SQLException e ) {
if ( LOG.isDebugEnabled() ) { LOG.debugf( "Unable to release JDBC result set [%s]", e.getMessage() );
LOG.debugf( "Unable to release result set [%s]", e.getMessage() );
}
} }
catch ( Exception e ) { catch ( Exception e ) {
// sybase driver (jConnect) throwing NPE here in certain cases, but we'll just handle the // try to handle general errors more elegantly
// general "unexpected" case LOG.debugf( "Unable to release JDBC result set [%s]", e.getMessage() );
LOG.debugf( "Could not close a JDBC result set [%s]", e.getMessage() );
} }
} }
} }

View File

@ -28,7 +28,16 @@ import java.sql.ResultSet;
import java.sql.Statement; 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 * @author Steve Ebersole
*/ */
@ -40,10 +49,6 @@ public interface JdbcResourceRegistry extends Serializable {
*/ */
public void register(Statement statement); public void register(Statement statement);
public void registerLastQuery(Statement statement);
public void cancelLastQuery();
/** /**
* Release a previously registered statement. * Release a previously registered statement.
* *
@ -83,4 +88,16 @@ public interface JdbcResourceRegistry extends Serializable {
* After execution, the registry is considered unusable. * After execution, the registry is considered unusable.
*/ */
public void close(); 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();
} }

View File

@ -2193,12 +2193,8 @@ public final class SessionImpl extends AbstractSessionImpl implements EventSourc
@Override @Override
public SharedSessionBuilder connection() { public SharedSessionBuilder connection() {
return connection( this.shareTransactionContext = true;
session.transactionCoordinator return this;
.getJdbcCoordinator()
.getLogicalConnection()
.getDistinctConnectionProxy()
);
} }
@Override @Override
@ -2221,10 +2217,13 @@ public final class SessionImpl extends AbstractSessionImpl implements EventSourc
return flushBeforeCompletion( session.flushBeforeCompletionEnabled ); return flushBeforeCompletion( session.flushBeforeCompletionEnabled );
} }
/**
* @deprecated Use {@link #connection()} instead
*/
@Override @Override
@Deprecated
public SharedSessionBuilder transactionContext() { public SharedSessionBuilder transactionContext() {
this.shareTransactionContext = true; return connection();
return this;
} }
@Override @Override

View File

@ -44,81 +44,6 @@ import static org.junit.Assert.assertTrue;
* @author Steve Ebersole * @author Steve Ebersole
*/ */
public class SessionWithSharedConnectionTest extends BaseCoreFunctionalTestCase { 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 @Test
@TestForIssue( jiraKey = "HHH-7090" ) @TestForIssue( jiraKey = "HHH-7090" )
public void testSharedTransactionContextSessionClosing() { public void testSharedTransactionContextSessionClosing() {