| Summary: | ResourcesPlugin#stop doesn't do a FULL_SAVE which can lead to snapshot & report of unsaved changes on next startup | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | James Blackburn <jamesblackburn+eclipse> |
| Component: | Resources | Assignee: | Szymon Ptaszkiewicz <sptaszkiewicz> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P3 | CC: | daniel_megert, dirk.segelhorst, eclipse-bugs, john.arthorne, krzysztof.daniel, matthias.renz, pwebster, sptaszkiewicz, Szymon.Brandys |
| Version: | 3.7 | ||
| Target Milestone: | 4.5 M3 | ||
| Hardware: | PC | ||
| OS: | All | ||
| Whiteboard: | |||
| Bug Depends on: | |||
| Bug Blocks: | 444988, 445223 | ||
|
Description
James Blackburn
It has always worked this way by design. All the "save participant" bundles will be stopped by the time the resources plugin stops, so they would not be able to save their state if the save happened during resources shutdown. Also, saving is a long running operation, so showing progress during save is important. By the time the resources bundle stops, the UI is already gone so we can't show progress. So, perhaps we can provide some kind of protection like: if the workspace has never been saved, then print a message saying the application failed to save its state. But I would argue this is never the "correct" time to save the workspace state. If we did save at that point, then our state would not match the state of save participants since they can't possibly run at that point. Thanks for the comment. (In reply to comment #1) > Also, > saving is a long running operation, so showing progress during save is > important. By the time the resources bundle stops, the UI is already gone so we > can't show progress. In an headless app there may not be any UI at all. > So, perhaps we can provide some kind of protection like: if the workspace has > never been saved, then print a message saying the application failed to save > its state. But I would argue this is never the "correct" time to save the > workspace state. If we did save at that point, then our state would not match > the state of save participants since they can't possibly run at that point. Makes sense. I think there are a few issues that arise: 1) This contract is entirely implicit between the Workbench UI and core.resources. AFAICS it isn't documented anywhere. The UI even downcasts IWorkspace to Workspace and calls a version of #save which isn't published through the interface 2) Is there any guarantee that core.resources really is the last bundle to be stop()ed? In Bug 149121 where .snap files exists and the master table appears to go backwards. If there's an outstanding Snapshot job stop() will cause a SNAPSHOT_SAVE to occur. I clearly don't understand how bundles are stopped on shutdown... 3) Does the partial snapshot save enough state? In the CDT HeadlessBuilder IApp, we encountered an issue where on WS restart, the build configurations were missing from a project. (This is, of course, without doing a full save.) This and Bug 310793 make it hard for IApplications to correctly use the ResourcesPlugin without the Workbench. > 1) This contract is entirely implicit between the Workbench UI and > core.resources. AFAICS it isn't documented anywhere. The UI even downcasts > IWorkspace to Workspace and calls a version of #save which isn't published > through the interface This is totally bogus and I hadn't noticed this before. I have entered bug 348007 to clean this up. > 2) Is there any guarantee that core.resources really is the last bundle to be > stop()ed? In Bug 149121 where .snap files exists and the master table appears > to go backwards. If there's an outstanding Snapshot job stop() will cause a > SNAPSHOT_SAVE to occur. I clearly don't understand how bundles are stopped on > shutdown... Bundles are stopped in reverse-prerequisite order. I.e., any bundle that requires core.resources will be stopped before core.resources. It is actually possible for a job from a misbehaving bundle to be running after that bundle has been shutdown. It would not be able to load classes at that point, but if its classes are already loaded it can run. The job API prints a warning in the log when this happens (if the bundle is using a bare thread you will never notice it). > 3) Does the partial snapshot save enough state? In the CDT HeadlessBuilder > IApp, we encountered an issue where on WS restart, the build configurations > were missing from a project. (This is, of course, without doing a full save.) I think the snapshot should have enough information so that a full workspace refresh on startup will restore all state. > This and Bug 310793 make it hard for IApplications to correctly use the > ResourcesPlugin without the Workbench. Yes. People tend to mimic/copy the startup/shutdown code of IDEApplication as there are number of different things an application typically needs to do. I agree many applications fail to do these things (locking on start and saving on shutdown). All sounds good. I'll have a poke to see if I can reproduce the corruption / missing data. (In reply to comment #3) > I agree many applications fail to do these things (locking on start and saving on > shutdown). It's worse that this: we can't lock on startup without a large refactoring. core.resources #start is called before our HeadlessBuilder (our HB class refers to ResourcesPlugin + friends). So while we can assert that the lock isn't held, by that point it's too late as core.resources will already be running in two eclipse instances. This looks like a duplicate of bug 272648. (In reply to comment #5) > This looks like a duplicate of bug 272648. The symptoms described in bug 272648 are similar. Though I will mark bug 272648 as a duplicate of this one because of more details provided here. *** Bug 272648 has been marked as a duplicate of this bug. *** Problem occurs in another way, too: The user has a running Eclipse/RCP application and second RCP application instance is trying to lock the same workspace. This follows in a error message. Now the user closes the first running RCP app. After this, the user confirms the error message of the second RCP app ant the second instance shuts down the org.eclipse.resource-bundle which creates .snap files. The result is that the workspace can not be reconstructed on a new eclipse startup. *** Bug 438360 has been marked as a duplicate of this bug. *** I have been getting the "workspace exited with unsaved changes" message for quite some time during almost every Eclipse start, even if all previous sessions ended without crash. This means there is some very stubborn code (somewhere in one of these: SDK, EGit, Releng Tools, API Tools) that keeps trying to modify workspace after the user asked Eclipse to close. Since it happens only in my main workspace and never while debugging, the solution with breakpoints from bug 272648 comment 5 doesn't help. What we can do is to track all snapshot requests that happen after FULL_SAVE and then log them during ResourcesPlugin#stop. This is similar to the proposal from bug 272648 comment 6. If a plugin uses resources API, it must depend on core.resources plugin, and so the bundle.stop() will happen before ResourcesPlugin#stop (see comment 3). This will give us maximum time to catch any late workspace operation and enough information to distinguish it from a real crash. Fixed in master: http://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=0bb618ddd61532db5deb1f593e987e1debe88aa7 Here is an example of the entry log that will be produced during ResourcesPlugin#stop if the last workspace save is not FULL_SAVE: !ENTRY org.eclipse.core.resources 4 10035 2014-09-22 11:15:07.471 !MESSAGE The workspace will exit with unsaved changes in this session. !SUBENTRY 1 org.eclipse.core.resources 4 10035 2014-09-22 11:15:07.471 !MESSAGE Snapshot requestor: org.eclipse.core.tests.resources.session.TestBug347907$1(problematic job) !STACK 0 java.lang.RuntimeException: Snapshot requested at org.eclipse.core.internal.resources.SaveManager.rememberSnapshotRequestor(SaveManager.java:561) at org.eclipse.core.internal.resources.SaveManager.snapshotIfNeeded(SaveManager.java:1476) at org.eclipse.core.internal.resources.Workspace.endOperation(Workspace.java:1499) at org.eclipse.core.internal.resources.InternalWorkspaceJob.run(InternalWorkspaceJob.java:45) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54) !SUBENTRY 1 org.eclipse.core.resources 4 10035 2014-09-22 11:15:07.473 !MESSAGE Snapshot requestor: org.eclipse.core.internal.jobs.ThreadJob(Implicit Job) !STACK 0 java.lang.RuntimeException: Snapshot requested at org.eclipse.core.internal.resources.SaveManager.rememberSnapshotRequestor(SaveManager.java:561) at org.eclipse.core.internal.resources.SaveManager.snapshotIfNeeded(SaveManager.java:1476) at org.eclipse.core.internal.resources.Workspace.endOperation(Workspace.java:1499) at org.eclipse.core.internal.resources.File.create(File.java:175) at org.eclipse.core.internal.resources.File.create(File.java:188) at org.eclipse.core.tests.resources.session.TestBug347907$2.run(TestBug347907.java:64) at java.lang.Thread.run(Thread.java:853) Verified in I20140923-0800. *** Bug 185968 has been marked as a duplicate of this bug. *** |