If a ServletContextListener implmentation references a class that is not yet loaded, the loading would with some ZipException. This is caused by the fact that when a context is stopped, the classloader is unbound before the ServletContextListeners are called. The sequence of calls is something like this: StandardContext.stop() lifecycle.fireLifecycleEvent(STOP_EVENT, null); NamingContextListener.lifecycleEvent(); ContextBindings.unbindClassLoader() ... listenerStop(); // trouble My solutions was to move listenerStop() before the call to fireLifecycleEvents() I also filed bug 37262, about the datasources not being released when the context is stopped. These two are somehow related, I think the right sequence when stopping the context is: 1. call the listeners 2. release the datasources 3. unbind the classloader
Created attachment 16816 [details] StandardContext patch
A necessary clarification: This problem refers to ServletContextlistener. destroy()
ServletContextlistener.destroy() ??? or ServletContextlistener.contextDestroyed() What about ServletContextlistener.contextInitialized() and listenerStart(), would it be possible to review that too with your knowledge of whats going on. I suspect the calling sequences for all setup/teardown should be Last In, First Out. If your solution is verified to be correct, I think you might have just possibily found the root cause of a great many outstanding ThreadDeath related issues that have been bounced around on bugzilla recently. These are ThreadDeath's caused from listenerStop() execution where the class loader has been invalidated.
(In reply to comment #3) > ServletContextlistener.destroy() ??? or ServletContextlistener.contextDestroyed() > > > What about ServletContextlistener.contextInitialized() and listenerStart(), > would it be possible to review that too with your knowledge of whats going on. > > I suspect the calling sequences for all setup/teardown should be Last In, First Out. > > If your solution is verified to be correct, I think you might have just > possibily found the root cause of a great many outstanding ThreadDeath related > issues that have been bounced around on bugzilla recently. These are > ThreadDeath's caused from listenerStop() execution where the class loader has > been invalidated. Understanding things is good rather than simply writing BS. This is related to JNDI. The code as of right now is: // Stop our application listeners listenerStop(); // Clear all application-originated servlet context attributes if (context != null) context.clearAttributes(); // Stop resources resourcesStop(); if ((realm != null) && (realm instanceof Lifecycle)) { ((Lifecycle) realm).stop(); } if ((cluster != null) && (cluster instanceof Lifecycle)) { ((Lifecycle) cluster).stop(); } if ((logger != null) && (logger instanceof Lifecycle)) { ((Lifecycle) logger).stop(); } if ((loader != null) && (loader instanceof Lifecycle)) { ((Lifecycle) loader).stop(); }
I don't like your patch, but NamingContextListener could be adjusted to do its stuff in AFTER_STOP_EVENT, so that JNDI still works when stopping the listeners. If you test that and if it works, I can make the change.
I don't like the patch either, it is indeed more appropiate to move some code from STOP_EVENT to AFTER_STOP_EVENT. I'll test the patch and post the results. With my limited knowledge of the code base, I think the fix involves moving this whole block from STOP to AFTER_STOP: if (container instanceof Context) { ContextBindings.unbindClassLoader (container, container, ((Container) container).getLoader().getClassLoader()); } if (container instanceof Server) { namingResources.removePropertyChangeListener(this); ContextBindings.unbindClassLoader (container, container, this.getClass().getClassLoader()); } ContextAccessController.unsetSecurityToken(getName(), container); namingContext = null; envCtx = null; compCtx = null; initialized = false;
I did apply something very close to the proposed patch, but did not test it well.