Jackrabbit fails to shutdown properly when tomcat is shutting down

• Key: JCR-120
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Jukka Zitting
• Reporter: fabrizio giustina
• Component/s: jackrabbit-core, observation
• Fix Version/s: 1.0

Description

This is the same issue already discudded in http://issues.apache.org/jira/browse/JCR-57

The problem only occurs when Jackrabbit is deployed in the WEB-INF/lib directory of a web application in Tomcat.
During dispose() jackrabbit tries to instantiate a few objects from classes which were not previously loaded by the webapp classloader, but tomcat doesn't allow to load new classes while shutting down.
This causes the repository not to be closed properly, and an annoying set of stack traces are written to the log.

It seems that there are only two classes which are loaded in this situation: org.apache.jackrabbit.core.observation.EventListenerIteratorImpl and org.apache.jackrabbit.core.fs.FileSystemPathUtil. This is the log from the server standard output:

org.apache.catalina.loader.WebappClassLoader loadClass
INFO: Illegal access: this web application instance has been stopped already. Could not load org.apache.jackrabbit.core.observation.EventListenerIteratorImpl. The eventual following stack trace is caused by an error thrown for debugging purposes as well as to attempt to terminate the thread which caused the illegal access, and has no functional impact.
[repeaded more times at each shutdown]

org.apache.catalina.loader.WebappClassLoader loadClass
INFO: Illegal access: this web application instance has been stopped already. Could not load org.apache.jackrabbit.core.fs.FileSystemPathUtil. The eventual following stack trace is caused by an error thrown for debugging purposes as well as to attempt to terminate the thread which caused the illegal access, and has no functional impact.


A quick fix is to force preloading of classes normally needed only during shutdown, simply adding a static block to caller classes. The following patch makes tomcat happy, causing classes to be loaded by the webapp classloaded when still allowed (probably not really elegant, but perfectly working...)




Index: org/apache/jackrabbit/core/fs/FileSystemResource.java
===================================================================
--- src\java\org\apache\jackrabbit\core\fs\FileSystemResource.java (revision 169503)
+++ src\java\org\apache\jackrabbit\core\fs\FileSystemResource.java (working copy)
@@ -30,6 +30,11 @@
 
     protected final String path;
 
+ static {
+ // preload FileSystemPathUtil to prevent classloader issues during shutdown
+ FileSystemPathUtil.class.hashCode();
+ }
+
     /**
      * Creates a new <code>FileSystemResource</code>
      *
Index: org/apache/jackrabbit/core/observation/ObservationManagerImpl.java
===================================================================
--- src\java\org\apache\jackrabbit\core\observation\ObservationManagerImpl.java (revision 169503)
+++ src\java\org\apache\jackrabbit\core\observation\ObservationManagerImpl.java (working copy)
@@ -54,6 +54,11 @@
      */
     private final ObservationManagerFactory obsMgrFactory;
 
+ static {
+ // preload EventListenerIteratorImpl to prevent classloader issues during shutdown
+ EventListenerIteratorImpl.class.hashCode();
+ }
+
     /**
      * Creates an <code>ObservationManager</code> instance.
      *

Comments

[fabrizio giustina]

stack from error (1)

[fabrizio giustina]

stack from error (2)

[fabrizio giustina]

svn diff

[Jukka Zitting]

As discussed in JCR-57, this is a rather tricky question where the correct solution is not obvious. I'm a bit worried about the proposed solution as it will break as soon as the dispose() code gets refactored to use different classes.

What would be the best solution to this problem? I tried looking for a more generic solution, but couldn't find any.

[Jukka Zitting]

Fixed as suggested in revision 179394.

Please reopen this issue if a better workaround is found.

[Jukka Zitting]

Reopening this issue as it seems that the fix did get broken. :-( See http://article.gmane.org/gmane.comp.apache.jackrabbit.devel/3745 for Fabrizio's message that details the breakage.

I propose to remove the entire addShutdownHook() call from the RepositoryImpl class. The shutdown hook could possibly be set up by the BindableRepositoryFactory class and other similar container tools, but normal client code that instantiates a RepositoryImpl object should be required to also explicitly call RepositoryImpl.shutdown() to properly close the repository.

[Jukka Zitting]

Scheduled for 1.0 and changed from an observation improvement request to a core bug report.

[Stefan Guggisberg]

> I propose to remove the entire addShutdownHook() call from the RepositoryImpl class

+1

[Felix Meschberger]

 > I propose to remove the entire addShutdownHook() call from the RepositoryImpl class

+1

[Edgar Poce]

  > I propose to remove the entire addShutdownHook() call from the RepositoryImpl class

+1.

[Jukka Zitting]

Removed the shutdown hook in revision 307128. This change makes it the responsibility of the client application that instantiates the RepositoryImpl object to also invoke the shutdown method. However I did place a similar shutdown hook into the BindableRepository class in order to minimize the effect on JNDI environments where there is no easy way to explicitly invoke the shutdown method.

I'm closing this issue with this change. See JCR-245 and the related issues for further discussion on how to best manage repository shutdown.