Bug 33357 - DataSourceRealm leaks connections
Summary: DataSourceRealm leaks connections
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 5
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 5.5.7
Hardware: All Windows 2000
: P1 major (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
: 33526 33938 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-02-02 15:06 UTC by Dominik Drzewiecki
Modified: 2005-03-09 14:27 UTC (History)
2 users (show)



Attachments
Refactored o.a.c.realm.DataSourceRealm (18.14 KB, text/plain)
2005-02-03 01:20 UTC, Dominik Drzewiecki
Details
Modified o.a.c.realm.LocalStrings.properties (4.80 KB, text/plain)
2005-02-03 01:25 UTC, Dominik Drzewiecki
Details
o.a.c.realm.DataSourceRealm (18.62 KB, text/plain)
2005-02-03 10:56 UTC, Dominik Drzewiecki
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Dominik Drzewiecki 2005-02-02 15:06:11 UTC
o.a.c.r.DataSourceRealm leaks connections (does not return to the pool) in
getRoles(String). The new connection is obtained from the data source, but never
returned. Bedides there seems to be a slight performance optimization possible
so that the getRoles makes use of the very same connection the authenticate()
does. Right now the whole authentication process i.e. authentication and
retrieval of user roles (which I consider as a whole and non-separable) requires
*two* connections of which one is never returned. I observe the increase of the
number of database backend. It eventually reaches the limit rendering the Realm
unusable.

One might workaround it by adding the following attributes in her datasource
Resource definition: removeAbandoned="true" removeAbandonedTimeout="15". But for
heavy loaded applications which extensively use the Realm facility 15 seconds
might be way too much.

Here is what logAbandoned="true" produced:

DBCP object created 2005-02-02 14:40:38 by the following code was never closed:
java.lang.Exception
   at
org.apache.tomcat.dbcp.dbcp.AbandonedTrace.setStackTrace(AbandonedTrace.java:157)
   at
org.apache.tomcat.dbcp.dbcp.AbandonedObjectPool.borrowObject(AbandonedObjectPool.java:76)
   at
org.apache.tomcat.dbcp.dbcp.PoolingDataSource.getConnection(PoolingDataSource.java:95)
   at
org.apache.tomcat.dbcp.dbcp.BasicDataSource.getConnection(BasicDataSource.java:540)
   at org.apache.catalina.realm.DataSourceRealm.open(DataSourceRealm.java:407)
   at org.apache.catalina.realm.DataSourceRealm.getRoles(DataSourceRealm.java:538)
   at
org.apache.catalina.realm.DataSourceRealm.authenticate(DataSourceRealm.java:360)
   at
org.apache.catalina.realm.DataSourceRealm.authenticate(DataSourceRealm.java:284)
   at
org.apache.catalina.authenticator.FormAuthenticator.authenticate(FormAuthenticator.java:256)
   at
org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:391)
   at org.apache.catalina.valves.AccessLogValve.invoke(AccessLogValve.java:526)
   at org.apache.catalina.authenticator.SingleSignOn.invoke(SingleSignOn.java:365)
   at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:126)
   at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:105)
   at
org.apache.catalina.valves.FastCommonAccessLogValve.invoke(FastCommonAccessLogValve.java:481)
   at
org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:107)
   at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:148)
   at org.apache.coyote.http11.Http11Processor.process(Http11Processor.java:825)
   at
org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler.processConnection(Http11Protocol.java:738)
   at
org.apache.tomcat.util.net.PoolTcpEndpoint.processSocket(PoolTcpEndpoint.java:526)
   at
org.apache.tomcat.util.net.LeaderFollowerWorkerThread.runIt(LeaderFollowerWorkerThread.java:80)
   at
org.apache.tomcat.util.threads.ThreadPool$ControlRunnable.run(ThreadPool.java:684)
   at java.lang.Thread.run(Unknown Source)

I volunteer to fix this bug as well as rework the DataSourceRealm which seems a
bit messy to me IMHO. Particularly the fact that two connections are reqired is
the most annoying.
Comment 1 Remy Maucherat 2005-02-02 16:17:39 UTC
Didn't you mention the problem earlier in another report ? I remember something
like this, but didn't see anything obvious in the code (which I'm not too
familiar with).

Yes, refactoring and removing the need for using two connections would be
useful. Be careful not to introduce new bugs, though ;)
Comment 2 Dominik Drzewiecki 2005-02-02 20:07:46 UTC
(In reply to comment #1)
> Didn't you mention the problem earlier in another report ? I remember something
> like this, but didn't see anything obvious in the code (which I'm not too
> familiar with).
You might have confused it with this enhancement proposal:
http://issues.apache.org/bugzilla/show_bug.cgi?id=33266

Both issues emerged while trying to setup a self-contained webapp (with its own
datasource, realm and database driver in WEB-INF/lib). The one decribed in this
report is however far more severe. 
Comment 3 Dominik Drzewiecki 2005-02-03 01:20:36 UTC
Created attachment 14165 [details]
Refactored o.a.c.realm.DataSourceRealm
Comment 4 Dominik Drzewiecki 2005-02-03 01:25:09 UTC
Created attachment 14166 [details]
Modified o.a.c.realm.LocalStrings.properties

Fixed two message keys for more consistent look (uppercesed 's' in a word
datasourceRealm). The rest of resource bundles do not contain fixed keys.
Comment 5 Remy Maucherat 2005-02-03 01:38:31 UTC
Why not ...

Do you know the purpose of the
            if( !dbConnection.getAutoCommit() ) {
                dbConnection.commit();             
            }
which was in the code before ? I do not see any writes being made.
Comment 6 Dominik Drzewiecki 2005-02-03 10:28:57 UTC
(In reply to comment #5)
> Why not ...
> 
> Do you know the purpose of the
>             if( !dbConnection.getAutoCommit() ) {
>                 dbConnection.commit();             
>             }
> which was in the code before ? I do not see any writes being made.

I'll look into it (by forcing autoCommit off) and let you know if it might have
any impact.

Comment 7 Dominik Drzewiecki 2005-02-03 10:56:24 UTC
Created attachment 14168 [details]
o.a.c.realm.DataSourceRealm

Indeed, according to jdbc specs, some resources (locks particularly) might not
get released when leaving the connection uncommited. I added the required block
back in a single place, just before closing the dbConnection (or should I say
returning to the pool)
Comment 8 Remy Maucherat 2005-02-05 11:36:58 UTC
I have applied your patch.
Comment 9 Mark Thomas 2005-02-11 21:39:35 UTC
*** Bug 33526 has been marked as a duplicate of this bug. ***
Comment 10 Remy Maucherat 2005-03-09 23:27:06 UTC
*** Bug 33938 has been marked as a duplicate of this bug. ***