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 committed Mar 15, 2012
1 parent 476fd2f commit ee9b358
Show file tree
Hide file tree
Showing 3 changed files with 197 additions and 59 deletions.
Expand Up @@ -26,6 +26,7 @@
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 @@ -174,72 +175,20 @@ public void releaseResources() {
private void cleanup() {
for ( Map.Entry<Statement,Set<ResultSet>> entry : xref.entrySet() ) {
if ( entry.getValue() != null ) {
for ( ResultSet resultSet : entry.getValue() ) {
close( resultSet );
}
entry.getValue().clear();
closeAll( entry.getValue() );
}
close( entry.getKey() );
}
xref.clear();

for ( ResultSet resultSet : unassociatedResultSets ) {
close( resultSet );
}
unassociatedResultSets.clear();

// TODO: can ConcurrentModificationException still happen???
// Following is from old AbstractBatcher...
/*
Iterator iter = resultSetsToClose.iterator();
while ( iter.hasNext() ) {
try {
logCloseResults();
( ( ResultSet ) iter.next() ).close();
}
catch ( SQLException e ) {
// no big deal
log.warn( "Could not close a JDBC result set", e );
}
catch ( ConcurrentModificationException e ) {
// this has been shown to happen occasionally in rare cases
// when using a transaction manager + transaction-timeout
// where the timeout calls back through Hibernate's
// registered transaction synchronization on a separate
// "reaping" thread. In cases where that reaping thread
// executes through this block at the same time the main
// application thread does we can get into situations where
// these CMEs occur. And though it is not "allowed" per-se,
// the end result without handling it specifically is infinite
// looping. So here, we simply break the loop
log.info( "encountered CME attempting to release batcher; assuming cause is tx-timeout scenario and ignoring" );
break;
}
catch ( Throwable e ) {
// sybase driver (jConnect) throwing NPE here in certain
// cases, but we'll just handle the general "unexpected" case
log.warn( "Could not close a JDBC result set", e );
}
}
resultSetsToClose.clear();
closeAll( unassociatedResultSets );
}

iter = statementsToClose.iterator();
while ( iter.hasNext() ) {
try {
closeQueryStatement( ( PreparedStatement ) iter.next() );
}
catch ( ConcurrentModificationException e ) {
// see explanation above...
log.info( "encountered CME attempting to release batcher; assuming cause is tx-timeout scenario and ignoring" );
break;
}
catch ( SQLException e ) {
// no big deal
log.warn( "Could not close a JDBC statement", e );
}
protected void closeAll(Set<ResultSet> resultSets) {
for ( ResultSet resultSet : resultSets ) {
close( resultSet );
}
statementsToClose.clear();
*/
resultSets.clear();
}

public void close() {
Expand Down Expand Up @@ -296,6 +245,7 @@ protected void close(ResultSet resultSet) {
InvalidatableWrapper<ResultSet> wrapper = (InvalidatableWrapper<ResultSet>) resultSet;
close( wrapper.getWrappedObject() );
wrapper.invalidate();
return;
}

try {
Expand All @@ -306,5 +256,10 @@ protected void close(ResultSet resultSet) {
LOG.debugf( "Unable to release 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() );
}
}
}
60 changes: 60 additions & 0 deletions hibernate-core/src/test/java/org/hibernate/IrrelevantEntity.java
@@ -0,0 +1,60 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* Copyright (c) 2012, Red Hat Inc. or third-party contributors as
* indicated by the @author tags or express copyright attribution
* statements applied by the authors. All third-party contributions are
* distributed under license by Red Hat Inc.
*
* This copyrighted material is made available to anyone wishing to use, modify,
* copy, or redistribute it subject to the terms and conditions of the GNU
* Lesser General Public License, as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
* for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this distribution; if not, write to:
* Free Software Foundation, Inc.
* 51 Franklin Street, Fifth Floor
* Boston, MA 02110-1301 USA
*/
package org.hibernate;

import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.Id;

import org.hibernate.annotations.GenericGenerator;

/**
* A testing entity for cases where the entity definition itself is irrelevant (testing JDBC connection semantics, etc).
*
* @author Steve Ebersole
*/
@Entity
public class IrrelevantEntity {
private Integer id;
private String name;

@Id
@GeneratedValue( generator = "increment" )
@GenericGenerator( name = "increment", strategy = "increment" )
public Integer getId() {
return id;
}

public void setId(Integer id) {
this.id = id;
}

public String getName() {
return name;
}

public void setName(String name) {
this.name = name;
}
}
@@ -0,0 +1,123 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* Copyright (c) 2012, Red Hat Inc. or third-party contributors as
* indicated by the @author tags or express copyright attribution
* statements applied by the authors. All third-party contributions are
* distributed under license by Red Hat Inc.
*
* This copyrighted material is made available to anyone wishing to use, modify,
* copy, or redistribute it subject to the terms and conditions of the GNU
* Lesser General Public License, as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
* for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this distribution; if not, write to:
* Free Software Foundation, Inc.
* 51 Franklin Street, Fifth Floor
* Boston, MA 02110-1301 USA
*/
package org.hibernate.sharedSession;

import org.hibernate.Criteria;
import org.hibernate.IrrelevantEntity;
import org.hibernate.Session;
import org.hibernate.engine.spi.SessionImplementor;

import org.junit.Test;

import org.hibernate.testing.FailureExpected;
import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

/**
* @author Steve Ebersole
*/
public class SessionWithSharedConnectionTest extends BaseCoreFunctionalTestCase {
@Test
@TestForIssue( jiraKey = "HHH-7020" )
@FailureExpected( jiraKey = "HHH-7020" )
public void testSharedTransactionContextSessionClosing() {
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 testSharedTransactionContextAutoClosing() {
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() );
}

@Override
protected Class<?>[] getAnnotatedClasses() {
return new Class[] { IrrelevantEntity.class };
}
}

0 comments on commit ee9b358

Please sign in to comment.