| Summary: | Eclipse and JVM Shutdown Hooks | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Tom Musta <tmusta> | ||||||
| Component: | Runtime | Assignee: | Thomas Watson <tjwatson> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | jeffmcaffer, pascal | ||||||
| Version: | 3.1 | ||||||||
| Target Milestone: | 3.1 M7 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | 88915 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
need to investigate for M6 as part of the classloading work. Yes, this does appear to be the same issue documented in bug 73059. I will look further into preloading the set of classes needed by the shutdown hook but in general this will not be practical because parts of the implementations come from 3rd party code. It will be very brittle. I agree, I seem to remember that we did not close the classloaders on shutdown. Jeff, does this not sound familiar? I notice that you are using Eclipse-AutoStart: true I think if you use some of the advanced features of the Eclipse-AutoStart header then you can filter some packages to prevent your bundle from being auto-started. For example the packages with your shutdown hooks should not cause your bundle to be auto-started. Jeff or Rafael, can you point Tom to some documentation about the Eclipse-AutoStart header? There is an "exceptions" attribute (TJ, should this be a directive?). Basically you set Eclipse-AutoStart to the base behaviour (true or false) and then specify exception packages which cause the opposite behaivour. So in Eclipse-AutoStart: true;exceptions="foo" the bundle will be autostarted for loads of all classes in all packages *except* the foo package. Technically it should be a directive. I'd hate to have to convert now. This has been available since 3.0 ... Is there and value in converting this to a directive? Tooling would be effected too ... ok, lets not change it. on the classloader closing, I don't remember. There must be classloaders being closed or disabled because classloads are failing. We need to recapture the logic used here. see bug 47255. That explains that we do not close classloaders on shutdown. I think the classloader error occurs if the bundle has Eclipse-AutoStart: true. this causes the EclipseClassLoader to prevent classloads if the bundle is stopped. Cool. so this means that Tom M can either turn off Autostart or list the classes involved in shutdown as exceptions. The bummer would be if for some reason he needed autostart to be triggered by loading of classes from the same package as the shutdown code. I've tried flipping Auto-Start to false in my simple example and it does not seem to have any effect (R3.1M5a). What I'm seeing in the debugger is that all bundle loaders are being closed as the framework shuts down: Thread [System Bundle Shutdown] (Suspended (breakpoint at line 251 in BundleLoader)) BundleLoader.close() line: 251 BundleHost.closeBundleLoader(BundleLoaderProxy) line: 558 BundleHost.refresh() line: 132 StartLevelManager.unloadAllBundles(BundleRepository) line: 699 StartLevelManager.decFWSL(int) line: 617 StartLevelManager.doSetStartLevel(int, AbstractBundle) line: 284 StartLevelManager.shutdown() line: 257 SystemBundle.suspend() line: 190 Framework.shutdown() line: 420 SystemBundle$1.run() line: 171 Thread.run() line: 568 The shutdown hook is triggered by the System.exit() in the org.eclipse.core.launcher.Main which occurs after framework shutdown. At this point, the BundleLoader will not allow any additional classes to be loaded. Something has changed since 3.0 that reintroduced this problem. I will investigate. This is not a regression. The same bug is in 3.0.1. We are closing the BundleLoaders on shutdown which is causing this problem. hmmm. I was pretty sure we were closing them but I'm getting pretty old... So what to do? some sort of indication that they should not be closed? and what about reactivation? The simple fix is to just not unload all the bundles on shutdown. I believe that was the original intent of bug 47255. But that was not fixed properly. I tested this out, and it seems to fix the problem but it opens us up to other issues. For example, at shutdown the frameworkLog is set to null. This will cause NPE if any logging is needed while loading the classes in the shutdown hooks. I ran into an NPE when the plugin had autostart set to true because the EclipseClassLoader wanted to log an error ... there has to be an end to the expectations. I mean if someone registers shutdown code, the code has to be pretty darn careful about what it does as the system is going down so various parts are being torn apart. TomM, do you have a sense as to what kinds of things are needed during this shutdown code? for example, can we say that it will not reference any OSGi classes? will only load classes from its own bundle (and the system classloader)? we have to put a box around it somehow. before fixing this, we need to clean up the open files we are leaving arround on shutdown ... We encountered several issues while investigating this. Most having to do with enabling classloaders to function after the framework has shutdown. Several NPE's started to occur if the classloader was used after shutdown. Will look into implementing a solution in M7. Created attachment 19872 [details]
Proposed solution
This patch allows existing bundle classloaders to load classes even after the
framework has been shutdown. The bundle classloader and BundleLoader objects
can still function and use the proper OSGi delegation model. Note that all
references to the BundleLoader and bundle classloader objects have been dropped
by the framework. In theory this would allow a classloader to be GC'ed if all
classes it loaded have been GC'ed.
In the shutdown hook case this would not happen because an object would still
exist for the actual shutdown hook. Also all resources (jar files) have been
closed for the bundles and their classloaders upon shutdown. If a class is
loaded after the framework has been shutdown then the jars will be reopened on
demand. Potentially new classloaders could even be created in cases where an
existing BundleLoader has not created the classloader yet. I tested these
different scenarios out and they all worked as expected.
Please note that you must not use Eclipse-AutoStart: true in any of the bundles
which contain code that may run in your shutdown hook. The EclispeClassLoader
will prevent any classes from being loaded after shutdown in bundles marked
with the Eclipse-AutoStart flag.
Jeff, can you review the patch. Thanks.
Created attachment 19884 [details]
updated patch
Had to release a fix to BundleLoader for dynamic imports. This is a new patch
against the latest in HEAD.
including Pascal for the review because he loves classloaders as much as I do :-) I reviewed the patch and it seems to contain the refactorings to use bit masks instead of booleans and the fix for this bug. For clarity we should release them independently. In BundleLoader.close() shouldn't we keep the "null'ation" of classloader and bundle, and skip them when a system property is enabled? I moved to using bit masks because I did not want to introduce yet another boolean slot to indicate the BundleLoader is closed. Previously the code checked if the bundle == null to see if the BundleLoader was closed. I can release the fix to move to bit masks before releasing the rest of the fix. We no longer null out any of the slots in BundleLoader during shutdown. This is to allow the BundleLoader to remain functional. We do remove all references to the BundleLoaderProxy's. All access to BundleLoaders (and BundleClassLoaders) from the framework is done through the BundleLoaderProxy. Keeping a refernce to the BundleClassLaoder from the BundleLoader should not prevent the BundleClassLoader from being GC'ed as long as no references exist to the BundleLoaderProxy. I released the patch in HEAD without splitting the concern. |
We are trying to port legacy runtime environments onto the Eclipse platform. Some of these environments use JVM shutdown hooks that perform some amount of cleanup. Unfortunately, in an Eclipse environment the bundles are stopped before the shutdown hooks are executed, which severely restricts what can be done in the hook. For example, no classes may be loaded. In general, we do not own the code for the hooks, so we cannot easily change the code to fit some other mechanism (e.g. bundle start levels). We can, however, live without a standard shutdown of the Eclipse platform. That is, if bundles stayed active during the JVM shutdown, I believe that would be acceptable. Here is a very trivial example. If you run it, you should get the following output: osgi> ### adding shutdown hook ### in shutdown hook com.acme.Main$ShutdownHook java.lang.NoClassDefFoundError: com/acme/Foo at com.acme.Main$ShutdownHook.run(Main.java:16) -------------- main application ----------- package com.acme; import org.eclipse.core.runtime.IPlatformRunnable; public class Main implements IPlatformRunnable { public Object run(Object arg0) throws Exception { System.out.println("### adding shutdown hook"); Runtime.getRuntime().addShutdownHook(new ShutdownHook()); return null; } private static class ShutdownHook extends Thread { public void run() { System.out.println("### in shutdown hook " + this.getClass().getName()); Foo f = new Foo(); } } } --------------- auxiliary class Foo -------------------- package com.acme; public class Foo { } -------------- plugin.xml ----------------------- <?xml version="1.0" encoding="UTF-8"?> <?eclipse version="3.0"?> <plugin> <extension id="Test" point="org.eclipse.core.runtime.applications"> <application> <run class="com.acme.Main"/> </application> </extension> </plugin> ------------------ manifest.mf ---------------- Manifest-Version: 1.0 Bundle-Name: Sample Bundle Bundle-SymbolicName: com.bugsrus Bundle-Version: 1.0.0 Bundle-ClassPath: testbundle.jar Bundle-Vendor: Acme Corp. Bundle-Localization: plugin Eclipse-AutoStart: true Import-Package: org.eclipse.core.runtime, org.osgi.framework