Community
Participate
Working Groups
The current code (eclipse 3.0 and before) handles shutdown with the following code snippet in the InternalBootLoader.run() method: public static Object run(String applicationName/*R1.0 compatibility*/, URL pluginPathLocation/*R1.0 compatibility*/, String location, String[] args, Runnable handler) throws Exception { .... try { result = runnable.run(applicationArgs); } catch (Throwable e) { e.printStackTrace(); throw new InvocationTargetException(e); } finally { shutdown(); } ... } The shutdown method is used to clean up the platform at shutdown. Among other things, it goes through and tells all plugins to shutdown and it also invokes the code to remove the workspace .lock file. The problem with this code is that the finally clause is not guaranteed to run. It will only run if the java code in the runnable.run() method exits normally or throws a Throwable. However, it is not guaranteed to run if the java run-time environment recieves a termination signal from the operating system. The symptoms of the problem are that plugins are not properly shutdown and the .lock file is left orphaned, causing the user to have to manually remove it to release the workspace. Granted, some terminations are impossible to service (such as a poweroff or a blue-screen event). However, there are many situations where the OS sends a signal to the runtime that could be handled gracefully - for example the user logs out or shuts down his computer gracefully, or sends an 'end process' command (for example from the Task Manager in Windows) or closes the -consolelog window, if that is open. The java runtime allows you to catch some of these signal events by registering a shutdown hook object with the Runtime using: Runtime.getRuntime().addShutdownHook(Runnable hook); When the runtime terminates, it will try to execute all Runnable objects that are registered this way. This will generally work fine in most runtime termination scenarios and is the preferred way to do final cleanup of temporary files and data at the end of a runtime session. It is true that even this is not a catch all, though. For example, if the user sends an 'end process' signal and then prematurely clicks on 'End Now' in the progress dialog, a long-duration shutdown hook may be interrupted prematurely. Thus it is preferable that the hook not do any activities that take a long time to complete. Given this, the preferred way to have the shutdown() method invoked in the InternalBootLoader.run() method, above, would be to create a Runnable that invokes it like so: public static Object run(String applicationName/*R1.0 compatibility*/, URL pluginPathLocation/*R1.0 compatibility*/, String location, String[] args, Runnable handler) throws Exception { .... Runnable shutDownHook= new Runnable(){ public void run(){ InternalBootLoader.shutdown(); } } Runtime.getRuntime().addShutdownHook(shutDownHook); try { result = runnable.run(applicationArgs); } catch (Throwable e) { e.printStackTrace(); throw new InvocationTargetException(e); } ... } A variation on this might be to use redundant shutdown paths (i.e. keep the finally clause in addition to the shutdown hook) that do different things based on the termination scenario and check state to avoid redundant or conflicting actions. However, I don't think that is necessary with this case. The above is the simplest fix to address the problem as it is presented in the platform right now. It is, however inelegant in general to separate the cleanup responsibility for a temporal resource so far from the creation of the resource. A better design/fix would put responsibility for cleanup of each such resource in need of it back in the class responsible for creation. For example, since it is the PlatformConfiguration class that creates the .lock file, it would be proper for that class to register a shutdown hook specifically to delete the .lock file. I'm not certain, but I think File.deleteOnExit() does this for you so at a minimum that method should be invoked when the file is created.
Can you describe a user scenario right now where the user has to manually delete the .lock file? We don't fail startup if the lock file exists, but only if we fail to open an exclusive channel on the lock file (using a java.nio locking channel).
Two things: 1) I see now that the use of nio locking channel is new and differs from 2.x, so that's good that it no longer blocks startup. I'm still getting used to the various changes in 3.0. I note in a few experiments that the .lock file is not ever deleted (3.0 M4) and looking at the code, its not clear it is even needed anymore. 2) the fundamental problem is still the same - the finally{} clause is not executed if the runtime terminates in response to an external signal. The remaining 'shutdown()' activities will not be carried out. I haven't done a detailed analysis to assertain whether any of the remaining shutdown activities are potentially data critical. My assumption has been that they are (better safe than sorry :-).
As you said yourself, there is no mechanism that is guaranteed to run in all crash cases, so adding this hook would add false security. I would suggest that the most common crashes are VM crashes, in which case the shutdown hook will likely not run. In any event, there is no critical platform data persisted during plugin shutdown. Lumping all plugin shutdowns into a shutdown hook in case a client plugin has critical data to persist doesn't make sense. We can't control how much time shutdown methods take, so we would be increasing the chance of the user losing patience and killing the process without waiting for clean termination.
I don't expect one to be able to catch ALL terminations, but one should be able to catch all normal terminations. We have a very common usecase that involves the platform being launched at user log-on and it is expected that it will be up all the while that the user is logged in. In this scenario, the common way a user will 'exit' the platform is by logging off his/her account. In this case, the JRE will recieve a termination signal from the operating system. This is NOT a VM crash. The user has every right to expect the code to respond appropriately to such a signal. With eclipse 2.1, it does not. With Eclipse 3 we have a chance to do this correctly. Note that a shutdownhook is called even with _normal_ application exit as well so it is appropriate to put normal shutdown code there. Shutdown code that takes a long time is at risk for premature termination only in the scenario where an external signal has been sent to the program and, if the code takes a long time so that the user is prompted if he wants to 'end now' and he is impatient. That scenario is most likely only if the user has selected to explicitely kill the process such as from the Task Manager. None of that would happen on a normal application exit and on a typical account logout the user would likely be patient since users are used to applications needing to save data when they quit. Plugin shutdowns should only be invoked during a shutdownhook if they have an implementation. Also, it may make sense to register each plugin shutdown method with its own shutdown hook.
I apologize for closing this bug a bit hastily. It looked like you were trying to solve a problem that didn't really exist. However, keep in mind that the application typically does all essential shutdown work before exiting. Plugin shutdown methods often do very little interesting work. For example, with the Workbench (IDE) application, persisting the UI and workspace state is done before it exits. If you are concerned about these cases you should enter a similar bug report with the provider of the application (such as Platform UI). If you are writing your own application, you may not be concerned about this since you can handle it yourself.
Also, another alternative would be to add a shutdown hook yourself whose only action would be to ask the application to exit (for instance, PlatformUI.getWorkbench().close()). That would cause the platform to shutdown normally (if there is enough time). Of course, one would have to avoid doing that during normal shutdown, but that would be easy to ensure. This wouldn't require any changes to Eclipse, plug-ins' shutdown methods or to the application itself.
First off, I don't think plugin code should ever take responsibility for shutting down the platform. That is just plain asking for trouble. Lets break the problem down like so: a) a java application can be terminated in one of the following ways: 1) code runs naturally to finish of main() execution. In the platform's case, this corresponds to the natural termination of the runnable.run(applicationArgs) invocation in the snippet above. In this scenario, the main thread continues, surrounding finally{} clauses are executed and shutdown hooks are executed. 2) code invokes System.exit(int). Note that this can happen at any point and if so, the main thread is terminated, finally{} clauses are ignored but shutdown hooks are executed. 3) JRE recieves a friendly termination signal from the OS -> this is typically what happens when the user 'logs out' of his OS account and is quite normal. In this scenario, the main thread is terminated, finally{} clauses are ignored but shutdown hooks are executed. 4) Catastrophic termination: really bad application crash / VM crash / unfriendly process termination / OS crash / power outage / God descends from the heavens. In this scenario, the main thread is terminated, finally{} clauses are ignored and shutdown hooks are probably ignored. So if you look across the 4 possible termination scenarios, we see that for covering our patooties, the finally{} clause is good for 1 out of 4, while shutdown hooks are good for 3 out of 4 (and may even still make it in some application or VM crash scenarios). b) Next, we ask the question: Is there anything that the platform really needs to make sure it puts a best effort at doing when the system shuts down? Any critical steps that need completion, or states that need to be resolved? If the answer is 'yes', then based on point (a) the preferred strategy is to use a shutdown hook. If the answer is 'no', then why do we have a finally{} clause here? c) Another question to ask is: What is the exact nature of the design-by-contract agreement between the platform and the Plugin.shutdown() method? Is this method guaranteed to be called when the platform shuts down? If so, then it should be invoked with a shutdown hook. {A strategy one might use is, upon loading a plugin, check to see if the shutdown() method has been overridden (i.e. is a declared method of the particular plugin class) and if so, register it in a shutdownhook, otherwise do not). If the answer is that plugin.shutdown() is NOT guaranteed to be called when the platform shuts down, then why do we even have this method? This is a critically important thing to be made clear here. If a plugin developer can not rely on the shutdown() method as a place to handle data critical shutdown actions, then he/she needs to know to instead register their own shutdown hook.
See bug 298416 for a related topic. We now have a shutdown hook that does a best effort to persist the framework state if the VM is shutdown unexpectedly. This bug is still requesting that the framework does a complete shutdown operation on VM termination. There are still concerns about doing long running operations in VM shutdown hooks.
No plans to add this to the framework. I know of several other users of the framework that implement their own hook to do this if they need it.
