Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HibernateJpaDialect does not allow ConnectionManager to eagerly release Connection for long-running Session [SPR-6895] #11560

Closed
spring-projects-issues opened this issue Feb 22, 2010 · 4 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Feb 22, 2010

Christopher G. Stach II opened SPR-6895 and commented

When JpaTransactionManager gets a handle for the Hibernate session's connection in doBegin with

ConnectionHandle conHandle = getJpaDialect().getJdbcConnection(em, definition.isReadOnly());

it never actually lets go. JpaTransactionManager eventually calls

getJpaDialect().releaseJdbcConnection(txObject.getConnectionHolder().getConnectionHandle(),
					txObject.getEntityManagerHolder().getEntityManager());

in doCleanupAfterCompletion, but HibernateJpaDialect has no implementation for releaseJdbcConnection.

Because this borrowed connection handle is never released, o.h.jdbc.ConnectionManager never returns the connection to the pool.

This setup eventually turns into a connection leak with Spring Web Flow when Hibernate fetches more connections to instantiate lazy collections and such, but the connection handles in the pool never get closed. Even if your database times out the connections and your pool eventually figures that out, the handles are still out of the pool until then.

After turning on debug logging for o.h.jdbc.ConnectionManager, things look like this:

$ grep jdbc.ConnectionManager debug.log | cut -c22- | sort | uniq -c
1 c.ConnectionManager.getConnection(ConnectionManager.java:167)
1 c.ConnectionManager.openConnection(ConnectionManager.java:446)
160 DEBUG jdbc.ConnectionManager aggressively releasing JDBC connection
157 DEBUG jdbc.ConnectionManager opening JDBC connection
156 DEBUG jdbc.ConnectionManager releasing JDBC connection [ (open PreparedStatements: 0, globally: 0) (open ResultSets: 0, globally: 0)]
1 DEBUG jdbc.ConnectionManager skipping aggresive-release due to borrowed connection

156 + 1 skipped release (and the two lines from the stack trace when the connection times out in the connection pool) = 157 opens.

$ grep jdbc.ConnectionManager debug.log | cut -c22- | sort | uniq -c
3 c.ConnectionManager.getConnection(ConnectionManager.java:167)
3 c.ConnectionManager.openConnection(ConnectionManager.java:446)
256 DEBUG jdbc.ConnectionManager aggressively releasing JDBC connection
251 DEBUG jdbc.ConnectionManager opening JDBC connection
248 DEBUG jdbc.ConnectionManager releasing JDBC connection [ (open PreparedStatements: 0, globally: 0) (open ResultSets: 0, globally: 0)]
3 DEBUG jdbc.ConnectionManager skipping aggresive-release due to borrowed connection

Again, 248 + 3 = 251.

Now, I don't see a need to have this JDBC connection even associated with the transaction as I do everything through JPA. I may be wrong about that, but it seems to only be used in this one spot. I went ahead and subclassed HibernateJpaDialect and made getJdbcConnection return null, just like DefaultJpaDialect. Things seem to be working, also (two separate runs):

$ grep jdbc.ConnectionManager debug.log | cut -c22- | sort | uniq -c
99 DEBUG jdbc.ConnectionManager aggressively releasing JDBC connection
95 DEBUG jdbc.ConnectionManager opening JDBC connection
95 DEBUG jdbc.ConnectionManager releasing JDBC connection [ (open PreparedStatements: 0, globally: 0) (open ResultSets: 0, globally: 0)]

$ grep jdbc.ConnectionManager debug.log | cut -c22- | sort | uniq -c
165 DEBUG jdbc.ConnectionManager aggressively releasing JDBC connection
161 DEBUG jdbc.ConnectionManager opening JDBC connection
161 DEBUG jdbc.ConnectionManager releasing JDBC connection [ (open PreparedStatements: 0, globally: 0) (open ResultSets: 0, globally: 0)]

The numbers match up and I am no longer getting timed out connections in the pool, which means I also shouldn't be running out of connections.

This should probably either be fixed, or this whole JDBC connection transaction association should be optional.

This was also described at http://ksevindik.blogspot.com/2008_11_01_archive.html.


Affects: 3.0 GA, 3.0.1

Issue Links:

Referenced from: commits ef227c5

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Since the Connection does get released on Session close, this isn't really a bug in the regular case - i.e. transaction-scoped Sessions. However, for long-running Sessions, the present behavior is of course very undesirable. You got an important point there.

So as of Spring 3.0.2, HibernateJpaDialect obtains the JDBC Connection from the Hibernate Session on demand only, through a custom ConnectionHandle implementation - very much like the one that we have in OpenJpaDialect already. This should hopefully solve your problem since Connections are not obtained at all if you're not requesting it through some JDBC access code.

This will be available in tonight's 3.0.2 snapshot. Please give it an early try if you have the chance!

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Michael Pilquist commented

This fixed caused a regression issue when using Hibernate in an OSGi environment. I'm posting the fix here in case anyone else runs in to a similar situation.

After upgrading to Spring 3.0.2.RELEASE, some bundles that use JDBC and not Hibernate (assuming a data source that is associated with an entity manager factory bean) failed with exceptions like:

Caused by: java.lang.IllegalArgumentException: interface org.hibernate.jdbc.ConnectionWrapper is not visible from class loader
        at java.lang.reflect.Proxy.getProxyClass(Proxy.java:353)
        at java.lang.reflect.Proxy.newProxyInstance(Proxy.java:581)
        at org.hibernate.jdbc.BorrowedConnectionProxy.generateProxy(BorrowedConnectionProxy.java:91)
        at org.hibernate.jdbc.ConnectionManager.borrowConnection(ConnectionManager.java:186)
        at org.hibernate.jdbc.JDBCContext.borrowConnection(JDBCContext.java:134)
        at org.hibernate.impl.SessionImpl.connection(SessionImpl.java:387)
        at org.springframework.orm.jpa.vendor.HibernateJpaDialect$HibernateConnectionHandle.getConnection(HibernateJpaDialect.java:150)
        at org.springframework.jdbc.datasource.ConnectionHolder.getConnection(ConnectionHolder.java:152)
        at org.springframework.jdbc.datasource.DataSourceUtils.doGetConnection(DataSourceUtils.java:108)
        at org.springframework.jdbc.datasource.TransactionAwareDataSourceProxy$TransactionAwareInvocationHandler.invoke(TransactionAwareDataSourceProxy.java:214)
        at $Proxy161.prepareStatement(Unknown Source)

Prior to 3.0.2, this was not an issue because the connection was opened from HibernateJpaDialect.getJdbcConnection, where the thread context class loader was configured to be the classloader of the bundle containing the entity manager factory bean. To fix this issue, we extended HibernateJpaDialect and wrapped the ConnectionHandle such that the thread context classloader is manually set to the proper bundle classloader before calls to HibernateConnectionHandler.getConnection and releaseConnection. For example:

public class ExtendedHibernateJpaDialect extends HibernateJpaDialect
{
    private static final long serialVersionUID = -5984046002134921459L;
    
    @Override
    public ConnectionHandle getJdbcConnection(final EntityManager entityManager, final boolean readOnly) throws PersistenceException,
        SQLException
    {
        final ConnectionHandle connectionHandle = super.getJdbcConnection(entityManager, readOnly);
        
        return new ConnectionHandle()
        {
            @Override
            public Connection getConnection()
            {
                return ContextClassLoaderUtility.doWithContextClassLoaderUnchecked(getClass().getClassLoader(), new Callable<Connection>() {
                    @Override
                    public Connection call() throws Exception
                    {
                        return connectionHandle.getConnection();
                    }
                });
            }
            
            @Override
            public void releaseConnection(final Connection connection)
            {
                ContextClassLoaderUtility.doWithContextClassLoader(getClass().getClassLoader(), new Runnable() {
                    @Override
                    public void run()
                    {
                        connectionHandle.releaseConnection(connection);
                    }
                });
            }
        };
    }
}

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Michael,

I see, so this only becomes a problem with plain JDBC based bundles when they lazily grab one of those Connections which actually come from Hibernate - I suppose through @Transactional backed by a JpaTransactionManager.

Note that the underlying Hibernate issue has been fixed in 3.5; so upgrading to Hibernate 3.5 would probably help. I'll nevertheless consider adding a ContextClassLoader override for Hibernate <3.5...

Actually, the ContextClassLoader override for getConnection should be sufficient, shouldn't it - because that's the only place where Hibernate is actually building the proxy. Is there a specific need for wrapping releaseConnection as well?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Michael Pilquist commented

Hi Juergen,

Yes, your description captures the issue -- bundles using transactional data sources backed by JpaTransactionManager.

I'll definitely move on to Hibernate 3.5 as well. I didn't realize the underlying issue was fixed there.

I believe the TCCL is only necessary on getConnection. In the example, I included the call to releaseConnection just in case there were future changes in Hibernate on the call to release.

Thanks for the help!

Michael

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants