Bug 50991 - Data source is closed before contextDestroyed is executed. tomcat 7.0.11
Summary: Data source is closed before contextDestroyed is executed. tomcat 7.0.11
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 7.0.11
Hardware: All Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-29 08:13 UTC by Mark Shifman
Modified: 2011-03-31 09:25 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Shifman 2011-03-29 08:13:54 UTC
In my ServletContextListener contextDestroyed method I do a database update to clean up a table.  In tomcat 7.0.11, I get the error (see below) when I shutdown tomcat.
Mar 28, 2011 10:47:57 AM org.apache.catalina.core.ApplicationContext log
> INFO: ContextListener: contextDestroyed()
> Mar 28, 2011 10:47:57 AM org.apache.catalina.core.StandardContext listenerStop
> SEVERE: Exception sending context destroyed event to listener instance of class org.ycmi.listeners.contextListener
> java.lang.RuntimeException: java.sql.SQLException: Data source is closed
>         at org.ycmi.prot.ypresults.db.dbUtils.update(dbUtils.java:495)
>         at org.ycmi.listeners.contextListener.contextDestroyed(contextListener.java:58)

This looks like the DataSource is being closed before contextDestroyed is run so I can't do the clean up. This wasn't a problem in tomcat 6.

As noted by Mark Thomas, this may be related to
https://issues.apache.org/bugzilla/show_bug.cgi?id=25060
Comment 1 Mark Thomas 2011-03-29 11:26:13 UTC
When working on this, it is worth taking a look at Filip's suggestion:
http://markmail.org/message/av4if6mstsh6qw4n
Comment 2 Mark Thomas 2011-03-30 08:27:22 UTC
Thanks for the report. This has been fixed in 7.0.x and will be included in 7.0.12 onwards. I'll be looking at Filip's suggestion next.
Comment 3 Mark Thomas 2011-03-30 09:35:36 UTC
Filip's suggested enhancement will also be in 7.0.12
Comment 4 Konstantin Kolinko 2011-03-30 19:21:20 UTC
Regarding r1086950 two comments:

1. The following is printed in the logs now:

31.03.2011 0:01:57 org.apache.catalina.users.MemoryUserDatabase save
SEVERE: User database has been configured to be read only. Changes cannot be saved

That is because MemoryUserDatabase#close() method does exist and calls save() unconditionally.

2. In NamingResources#cleanUp():
> resource = ctxt.lookup(name);

If it is the first lookup for a ContextResource, or this resource is not a "singleton", this will create an instance of it. -> see (1) below

For "javax.sql.DataSource" resources it does not happen, because a lookup() is called elsewhere just after binding them, to register them with JMX -> see (2)
but for any others this might be a slight concern.

Unrelated note, looking at (2): registering DataSource with JMX makes sense only if it is a singleton. Otherwise we would not be managing the actual object. It is good that it is singleton by default.


It would be nice to perform cleanup only for the resources that have been already created (entry.type == NamingEntry.ENTRY), but I do not see how that can be implemented.   The NamingEntry class is something internal to org.apache.naming.


My proposal is the following:
1. Assume the default value of "close" only for the resources of type "javax.sql.DataSource".

That is, for any other resource types assume that the value is unset by default and do not perform lookup of the resource.

2. Regarding (2): add a comment to the singleton attribute that singleton=true is necessary for JMX management of connection pools.

3. Do not call save() in MemoryUserDatabase.close() if it is readonly.

------------------------------------------------
(1): In the sources:
ContextResource is represented by ResourceRef object (in NamingContextListener#addResource(ContextResource))

ResourceRef is represented by NamingEntry.REFERENCE (in NamingContext#bind(Name, Object, boolean)
NamingContext.bind() creates

Lookup is implemented in NamingContext.lookup(Name, boolean) and
(entry.type == NamingEntry.REFERENCE) branch is called.
------------------------------------------------
(2):  In the sources:
In NamingContextListener#addResource(ContextResource):
if ("javax.sql.DataSource".equals(ref.getClassName())) {
 -> lookup
------------------------------------------------
Comment 5 Filip Hanik 2011-03-30 19:29:49 UTC
(In reply to comment #4)

> 
> My proposal is the following:
> 1. Assume the default value of "close" only for the resources of type
> "javax.sql.DataSource".

Simplest would be that if closeMethod is not set, don't call close.
That way you're backwards compatible, and provide the functionality a user needs.
close is not a method on the javax.sql.DataSource interface

Filip
Comment 6 Mark Thomas 2011-03-31 09:25:03 UTC
Thanks for the review and feedback.

I have made the following changes for 7.0.12:
- disable by default be changing the default for closeMethod to null (r1087286)
- don't try and close non-singleton resources as it is pointless (r1087291)
- only register DataSources with JMX if they are singletons (r1087292)