Bug 50159 - JNDI context returns new datasource instance each request
Summary: JNDI context returns new datasource instance each request
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 7.0.4
Hardware: PC Windows XP
: P2 critical (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-26 15:46 UTC by Mark Watson
Modified: 2010-11-04 14:01 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Watson 2010-10-26 15:46:50 UTC
Requesting a javax.sql.DataSource via JNDI results in a new instance each time.

In my particular case I have defined a c3p0 connection pool as follows:

<Resource auth="Container"
   name="jdbc/mydb"
   description="My Database"
   factory="org.apache.naming.factory.BeanFactory"
   type="com.mchange.v2.c3p0.ComboPooledDataSource"
   driverClass="org.gjt.mm.mysql.Driver"
   jdbcUrl="jdbc:mysql://localhost:3306/mydb?autoReconnect=true"
   user="myUserName"
   password="myPassword"
   minPoolSize="5"
   maxPoolSize="50"
   acquireIncrement="1"
   idleConnectionTestPeriod="100"
   maxIdleTime="1800"
   preferredTestQuery="SELECT 1;"
/>

I am using a 3rd party library to access the datasource.  Each time a request is made I can see a new connection pool being created.  With a simple test I was also able to see that the instance being returned was different each time.  Test code:

Context initialContext = new InitialContext();
return ((DataSource)initialContext.lookup(this.dataSource)).getConnection();

When comparing the source code for org.apache.naming.NamingContext between 7.02 and 7.04 the following 4 lines of code were removed (line 808):

if (obj != null) {
   entry.value = obj;
   entry.type = NamingEntry.ENTRY;
}

Adding this code back into the class fixes the issue.
Comment 1 Mark Watson 2010-10-26 16:58:05 UTC
I just came across the issue (49978) where this was explicitly made to behave this way.  I personally require the previous behavior.  I believe this is also how Tomcat 6 behaved, so I imagine this could cause some headaches for people upgrading.  I will need to find a way to get around this (which may mean I end up maintaining my own copy of org.apache.naming.NamingContext).

Are there backwards compatibility concerns here?  I understand I can cache the DataSource instance returned to me, however I also use 3rd party libraries that request the DataSource (Hibernate, BIRT, etc).
Comment 2 Mark Watson 2010-10-26 17:32:28 UTC
Sorry, I referenced the wrong issue.  The issue which prompted the change from 7.0.2 to 7.0.4 is 49994.
Comment 3 Mark Thomas 2010-10-26 17:35:39 UTC
I've done a bit more digging and I'm leaning towards the following:
- shareable by default
- look at the res-sharing-scope to determine shareable or non-shareable

That will return Tomcat 7 to the previous behaviour but provide a way for folks that want non-shareable resources to configure them. I need to do some more reading of the spec to see if such an approach is in alignment with the spec.
Comment 4 david jencks 2010-10-26 19:33:10 UTC
the aharable/non-sharable attribute refers to the connections, not the datasource/connection factory.  The spec is very clear that unless clearly indicated otherwise or unless the object is immutable, each jndi lookup should return a new object.  There are some tests for this for datasources in the ee tck.

Sharable refers to how many connection handles can be associated with a managed connection (in the same thread and same transaction).  Sharable means that if you get multiple connections (in the same thread and tx) they can all be backed by the same managed connection.  non-sharable means each connection needs its own managed connection.  Typically you'd use non-sharable connections if you got the connection with some unique data such as username/pw that is different for each connection.  Some resource adapters also don't support sharable connections.

I have no problem relaxing this (IMO very silly) requirement but please don't misuse the sharable flag for it.
Comment 5 Mark Thomas 2010-10-27 04:29:06 UTC
David,

Thanks for the clarification. Reading that part of the spec and trying to figure out what it actually means always makes my head hurt.

I'll leave the shareable flag alone (as far as I can tell, Tomcat doesn't do anything with it).

Bug 49994 indicates there are at least some users that want the spec mandated behaviour whilst it appears to be the expectation of most that the same object is returned from multiple look-ups. Configuring this per context would be easy but I think per resource configuration would be better. I'll see if there is a way to make that happen. If nothing simple jumps put at me, I'll likely go with per context configuration for now and we can look at extending it to per resource if there is any demand for that.
Comment 6 Konstantin Kolinko 2010-10-27 11:43:40 UTC
(In reply to comment #5)
> I'll likely go with per context configuration for now

Do not forget about GlobalNamingResources resources that are in server.xml.
Comment 7 Mark Thomas 2010-11-04 14:01:51 UTC
Fixed in trunk with a new configuration option, singleton, for resource elements that defaults to the current 6.0.x behaviour.