Community
Participate
Working Groups
The code which starts the workbench save job does not wait for the previous one to be finished. As a result, more than one job could theoretically run in parallel (happened to me while debugging). This can cause a problem if the code it executes, is not thread-safe or locks resources (deadlock?).
There are two jobs with a similar name: the "Workbench Auto-Save Job" and "Workbench Auto-Save Background Job" job. The "Workbench Auto-Save Job" job is created only once and runs in the UI thread. The "Workbench Auto-Save Job" job creates a new job - "Workbench Auto-Save Background Job" that doesn't run in the UI thread so there could be more "Workbench Auto-Save Background Job" jobs at the same time (org.eclipse.ui.internal.Workbench.persistWorkbenchModel). That is reproducible when debugging. It may be that you have seen the "Workbench Auto-Save Background Job" jobs. The "Workbench Auto-Save Job" job should check if the "Workbench Auto-Save Background Job" job is started and create a new job only if it doesn't exist.
(In reply to comment #1) > There are two jobs with a similar name: the "Workbench Auto-Save Job" and > "Workbench Auto-Save Background Job" job. > The "Workbench Auto-Save Job" job is created only once and runs in the UI > thread. > > The "Workbench Auto-Save Job" job creates a new job - "Workbench Auto-Save > Background Job" that doesn't run in the UI thread so there could be more > "Workbench Auto-Save Background Job" jobs at the same time > (org.eclipse.ui.internal.Workbench.persistWorkbenchModel). > That is reproducible when debugging. > It may be that you have seen the "Workbench Auto-Save Background Job" jobs. I won't argue about how the job's are labelled ;-). This bug is about the SAVE job i.e. the job that actually does the work and saves the state. I think this should be clear, especially, when one looks at the code.
They don't lock or depend on state that can change underneath them and cause a problem (the second job operates entirely on a copy). But we should still probably limit them so they don't crash into one another. PW
I have created a patch : https://git.eclipse.org/r/36151
(In reply to Snjezana Peco from comment #4) > I have created a patch : https://git.eclipse.org/r/36151 Snjezana, I think your change could lead to UI freezes, if the save job is running very long. I think a safer way is to use the new JobGroup API with a pool size of one. I updated your review with the change. Could you have a look at let me know what you are thinking?
(In reply to Lars Vogel from comment #5) > (In reply to Snjezana Peco from comment #4) > > I have created a patch : https://git.eclipse.org/r/36151 > > Snjezana, I think your change could lead to UI freezes, if the save job is > running very long. I think a safer way is to use the new JobGroup API with a > pool size of one. I updated your review with the change. Could you have a > look at let me know what you are thinking? I have updated the patch.
Gerrit change merged: https://git.eclipse.org/r/36151 was merged. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a8a4fe3ce3e43aec6d5a6d310fab6084c5070b26
Thanks Snjezana for your work and persistence. Sorry for being slow in reviewing this one.
New Gerrit change created: https://git.eclipse.org/r/41523
https://git.eclipse.org/r/#/c/41523 fixes the issue without using JobGroup. For more details see the discussion on https://git.eclipse.org/r/#/c/36151/
Gerrit change https://git.eclipse.org/r/41523 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=2532fb16367068c5993176ccee739b8abbafeb06
Thanks Snjezana.
