Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 327262

Summary: Mylyn Task working sets cause loss of working set data on eclipse crash
Product: z_Archived Reporter: Carsten Reckord <reckord>
Component: MylynAssignee: Carsten Reckord <reckord>
Status: RESOLVED FIXED QA Contact:
Severity: major    
Priority: P3 CC: steffen.pingel
Version: 3.4Keywords: greatbug
Target Milestone: 3.4.3   
Hardware: All   
OS: All   
Whiteboard:
Bug Depends on: 327396    
Bug Blocks:    
Attachments:
Description Flags
Fix for TaskWorkingSetUpdater triggering a save
steffen.pingel: iplog+
additional change to avoid loading of working sets during shutdown
none
mylyn/context/zip none

Description Carsten Reckord CLA 2010-10-07 13:34:17 EDT
I am using Task working sets to group my tasks. This causes all working sets appearing after the last task working set in the working set model to be lost when eclipse isn't shut down cleanly.

Steps to reproduce:
- Create a Resource working set A
- Create a Task working set B
- Create another Resource working set C
- Take a look at .metadata/.plugins/org.eclipse.ui.workbench/workingsets.xml - the order of the working sets in there should be A, B, C
- Restart eclipse
- Kill eclipse hard without touching the working sets
- Restart eclipse
- Working Set C is gone

The cause of this is the following: 

On startup, while the WorkingSetManager is still in the process of loading the working sets from file, the Mylyn TaskWorkingSetUpdater triggers a save of the WorkingSetManager for every freshly loaded Task working set (see stack trace below). Since not all WorkingSets have been loaded at that point, after we are through with loading we end up with a persisted configuration including only the sets up to the last Task WorkingSet. Now, this is usually not a problem if eclipse can be closed cleanly, which triggers a save of the complete model. However, when eclipse freezes (as it is prone to do especially during startup) and has to be shut down hard, you end up with the incomplete model persisted at that point on next startup.

Thread [Thread-3] (Suspended (entry into method saveWorkingSetState in AbstractWorkingSetManager))	
	owns: HashMap<K,V>  (id=84)	
	WorkingSetManager(AbstractWorkingSetManager).saveWorkingSetState(IMemento) line: 453	
	WorkingSetManager(AbstractWorkingSetManager).saveState(File) line: 810	
	WorkingSetManager.saveState() line: 136	
	WorkingSetManager.workingSetChanged(IWorkingSet, String, Object) line: 158	
	WorkingSet(AbstractWorkingSet).fireWorkingSetChanged(String, Object) line: 132	
	WorkingSet.setElements(IAdaptable[]) line: 242	
	TaskWorkingSetUpdater.checkElementExistence(IWorkingSet) line: 130	
	TaskWorkingSetUpdater.add(IWorkingSet) line: 105	
	AbstractWorkingSetManager$12.run() line: 740	
	SafeRunner.run(ISafeRunnable) line: 42	
	WorkingSetManager(AbstractWorkingSetManager).addToUpdater(IWorkingSet) line: 736	
	WorkingSetManager(AbstractWorkingSetManager).internalAddWorkingSet(IWorkingSet) line: 242	
	WorkingSetManager(AbstractWorkingSetManager).restoreWorkingSetState(IMemento) line: 513	
	WorkingSetManager.restoreState() line: 109	
	WorkbenchPlugin.getWorkingSetManager() line: 609	
	Workbench.getWorkingSetManager() line: 1455	
	WorkbenchPage.restoreState(IMemento, IPerspectiveDescriptor) line: 3165	
	WorkbenchWindow.restoreState(IMemento, IPerspectiveDescriptor) line: 2219	
	Workbench.doRestoreState(IMemento, MultiStatus) line: 3612	
	Workbench.access$29(Workbench, IMemento, MultiStatus) line: 3554	
	Workbench$55.run() line: 2261	
	Workbench.runStartupWithProgress(int, Runnable) line: 1975	
	Workbench.restoreState(IMemento) line: 2259	
	Workbench.access$27(Workbench, IMemento) line: 2230	
	Workbench$50.run() line: 2093	
	SafeRunner.run(ISafeRunnable) line: 42	
	Workbench.restoreState() line: 2037	
	WorkbenchConfigurer.restoreState() line: 183	
	WorkbenchAdvisor$1.run() line: 781
Comment 1 Carsten Reckord CLA 2010-10-07 13:40:52 EDT
Created attachment 180443 [details]
Fix for TaskWorkingSetUpdater triggering a save

This patch makes the TaskWorkingSetUpdater avoid an unnecessary save on the workingset model if it didn't change anything. 

This seems to be working for the other implementations of IWorkingSetUpdater, althought theoretically, the problem could still occur if for some reason there were entries to be removed during startup.
Comment 2 Steffen Pingel CLA 2010-10-07 17:19:14 EDT
Thanks for the great description and patch! This has been a long standing problem and we never got to the bottom of it. I'll merge this shortly.

If understand correctly, it sounds like this is actually a problem in the platform and that saving of working sets during loading should be prevented or delayed to avoid saving partial state?
Comment 3 Carsten Reckord CLA 2010-10-08 04:56:37 EDT
Yes, that's right. Saving during load is pretty dangerous, because it is almost guaranteed to persist a partial state. 

Usually, no change should be made to a WorkingSet during load I guess, which is what my patch fixes for the task sets. But when that occurs, it would be good if the platform would handle that situation gracefully. One way to do that is to disable save during load and if there are save requests to save the whole model after loading. A more dirty solution would be to always trigger a save after the model was fully loaded...
Comment 4 Steffen Pingel CLA 2010-10-09 19:06:13 EDT
Created attachment 180554 [details]
additional change to avoid loading of working sets during shutdown
Comment 5 Steffen Pingel CLA 2010-10-09 19:06:16 EDT
Created attachment 180555 [details]
mylyn/context/zip
Comment 6 Steffen Pingel CLA 2010-10-09 19:10:02 EDT
Committed both patch to head and e_3_6_m_3_4_x branch. Thanks a lot for the fix, Carsten!