| Summary: | [Workbench] Auto-save not re-enabled when preference set from 0 to X | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Snjezana Peco <snjezana.peco> | ||||
| Component: | IDE | Assignee: | Platform UI Triaged <platform-ui-triaged> | ||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | daniel_megert, john.arthorne, manderse, mauromol, pwebster | ||||
| Version: | 4.3 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | stalebug | ||||||
| Attachments: |
|
||||||
|
Description
Snjezana Peco
> There are two issues related to #402912: I don't think it's related to bug 402912, which just fixed a leak, but rather a problem in the fix for bug 2369. > I am willing to create a patch if you want. That would be welcome! (In reply to comment #0) > There are two issues related to #402912: > > 1) If you set the interval to 0, you won't be able to change it without > restarting the workspace because the autoSaveJob isn't created (if the > preference is set when starting Eclipse) or isn't rescheduled (if it is set > to 0 using the General preferences page). Right, there's no preference listener needed to reactivate the Job after the interval has been set to 0. > > 2) If the interval != 0, Eclipse starts the autoSaveJob job that runs > forever. There isn't any such a job in Eclipse and this breaks our tests. This is a peculiarity of your tests, which now needs to be adjusted for 4.3. > Also, I suppose this preference would have to be 0 by default because that > is behaviour users expect. The default for this pref is 5 minutes. PW Created attachment 229570 [details]
A patch
The patch fixes all described issues.
(In reply to comment #2) > (In reply to comment #0) > > > > 2) If the interval != 0, Eclipse starts the autoSaveJob job that runs > > forever. There isn't any such a job in Eclipse and this breaks our tests. > > This is a peculiarity of your tests, which now needs to be adjusted for 4.3. > There isn't a job in Eclipse that runs forever. There was such a job in Mylyn that was fixed. Other contributors could have the same issue. (In reply to comment #3) > Created attachment 229570 [details] > A patch Snjezana, you can't set the default to 0, we deliberately set it to 5 minutes. Also, you wouldn't use a Thread, the only part that was missing was the preference listener to kickstart the job if it is not running. PW (In reply to comment #4) > There isn't a job in Eclipse that runs forever. Job is the correct framework to use here. While it's waiting, it is not consuming thread resources, and we want it to be waiting. PW See #198241. When using Kepler M6, the JobManager is never idle. If you still want to use a job, you would have to destroy the job when it is finished and create a new job when needed. (In reply to comment #7) > See #198241. > When using Kepler M6, the JobManager is never idle. But 1) the job isn't running and blocked (like mentioned in that bug) it's just re-scheduled for 5 minutes from now and 2) I don't expect the job manager to be idle if it's doing something. John, do we have a requirement that the job manager be idle? It would seem to me that if we activated the workspace refresh preference it would never be idle as well. PW We don't know why a job hasn't finished its work when it is in the waiting state. Maybe it wait for some resource to be released. Because of that, we have to wait for all jobs to be finished before an action (removing a temporary created resource, for instance). If you don't want to create a thread, I don't see a reason for not creating a new job when it is necessary. Especially because this job is a workspace job and locks the workspace all the time. > Snjezana, you can't set the default to 0, we deliberately set it to 5 minutes. We can create a workaround for this case as well as for other cases. See https://issues.jboss.org/browse/JBIDE-13973 (In reply to comment #9) > We don't know why a job hasn't finished its work when it is in the waiting > state. Maybe it wait for some resource to be released. You cannot assume that jobs you don't own are related to any resource work, as obviously that's not the case here. > If you don't want to create a thread, I don't see a reason for not creating > a new job when it is necessary. Especially because this job is a workspace > job and locks the workspace all the time. If it was a workspace job, it would only lock the workspace when it was actually running (not while waiting). But this job is a workbench job, it doesn't lock the workspace (it's UI related, not workspace related). > We can create a workaround for this case as well as for other cases. See > https://issues.jboss.org/browse/JBIDE-13973 Yes, plugin_customization.ini is the workaround I would suggest as well. PW > You cannot assume that jobs you don't own are related to any resource work, as > obviously that's not the case here. When we, for instance, create a test project, Eclipse starts builders, WTP starts validators, m2e its jobs ... that can read/change the resources of the created project. Some of those jobs can be filtered by a job family. We can't know what some job works with our resource. That's why JobUtils.waitForIdle() has been introduced. It waits for all jobs to finish their work. If we try to remove the project (in the tearDown method) while some of the jobs has a file of the project open, deleting will fail on Windows and Linux/Unix NFS share device. This issue happens randomly and it is difficult to fix it. Now, we have to treat autoSaveJob in a special way. > If it was a workspace job, it would only lock the workspace when it was > actually running (not while waiting). You are right. > Yes, plugin_customization.ini is the workaround I would suggest as well. It will work if you remove the autoSaveJob job when the saveInterval = 0 (as in the current code) and recreate it when a user changes the saveInterval. +100 for not having a feature in default eclipse that forces long wait on jobs finishing. I believe we are not the only one that has test that wait for the jobs to finish to avoid conflicts. We also disable certain jobs explicitly to avoid this (i.e. build and validations are often cause of issues when running tests because they are not build for speed/unitests) Using plugin_customization.ini is really not a sustainable way to disable this - we would have to put a plugn_customization.ini into every test plugin - configured it in Tycho AND in eclipse (even worse) to be used. Could we please get a system property or other way of ensuring this job is not going to be present since it is *not* something that makes sense to have during test runs. I've opened bug 405456 to allow tests to disable the auto-save job PW (In reply to comment #11) > When we, for instance, create a test project, Eclipse starts builders, WTP > starts validators, m2e its jobs ... that can read/change the resources of > the created project. > Some of those jobs can be filtered by a job family. > We can't know what some job works with our resource. > That's why JobUtils.waitForIdle() has been introduced. It waits for all jobs > to finish their work. I think you're misunderstanding the nature of this job. It saves the user interface layout state, it has nothing to do with resources and doesn't lock any resources. When a job is scheduled to run 5 minutes in the future, this has *no* impact on anyone. Creating a dedicated thread to do this as you suggest is not a scalable solution as it maintains a dedicated thread when it is not needed. Also IJobManager.isIdle() will return true if the scheduled run time for a job has not yet arrived, so your test utility method should not be blocked by this. (In reply to comment #14) > I think you're misunderstanding the nature of this job. It saves the user > interface layout state, it has nothing to do with resources and doesn't lock > any resources. When a job is scheduled to run 5 minutes in the future, this > has *no* impact on anyone. Creating a dedicated thread to do this as you > suggest is not a scalable solution as it maintains a dedicated thread when > it is not needed. Also IJobManager.isIdle() will return true if the > scheduled run time for a job has not yet arrived, so your test utility > method should not be blocked by this. After migration to Kepler M6, we have noticed the following test failures: java.lang.RuntimeException: Long running tasks detected: Workbench Auto-Save Job (class org.eclipse.ui.internal.Workbench$60) at org.jboss.tools.test.util.JobUtils.waitForIdle(JobUtils.java:47) at org.jboss.tools.test.util.JobUtils.waitForIdle(JobUtils.java:31) at org.jboss.tools.test.util.ResourcesUtils.importProjectIntoWorkspace(ResourcesUtils.java:324) at org.jboss.tools.test.util.ResourcesUtils.importProject(ResourcesUtils.java:87) at org.jboss.tools.test.util.ResourcesUtils.importProject(ResourcesUtils.java:70) at org.jboss.tools.test.util.ResourcesUtils.importProject(ResourcesUtils.java:61) at org.jboss.tools.test.util.ResourcesUtils.importProject(ResourcesUtils.java:213) at org.jboss.ide.eclipse.archives.test.model.DirectoryScannerTest.setUp(DirectoryScannerTest.java:50) The delay is an hour. The failures don't happen when we disable this job. There is something strange going on here. The workbench save job runs very quickly and it should not even be noticeable. I wonder if your loop in JobUtils is somehow blocking the job from executing and you actually have a deadlock here. Can you debug your test and see what is going on here. Is the job actually running for an hour, or maybe something else is running that is making isIdle return false. The problem isn't what the autoSaveJob job works, but that it runs forever. #405456 solves it. (In reply to comment #16) > There is something strange going on here. The workbench save job runs very > quickly and it should not even be noticeable. I wonder if your loop in > JobUtils is somehow blocking the job from executing and you actually have a > deadlock here. Can you debug your test and see what is going on here. Is the > job actually running for an hour, or maybe something else is running that is > making isIdle return false. The org.jboss.tools.test.util.JobUtils.waitForIdle(long, long) method prints all jobs when the delay is expired. There is only one job - the Workbench Auto-Save Job (class org.eclipse.ui.internal.Workbench$60). If our tests can cause a deadlock, that is yet another reason for the autoSaveJob not running forever. Other projects/jobs can face a similar deadlock that hasn't existed before. > Can you debug your test ...
I can't debug the tests because the failure happens randomly on Hudson.
It isn't reproducible locally.
(In reply to comment #17) > The problem isn't what the autoSaveJob job works, but that it runs forever. > #405456 solves it. But it doesn't run forever... it only runs very briefly every five minutes. When a job is scheduled to run five minutes in the future, the job manager continues to be idle until that job runs. In your scenario it appears the job manager is non-idle for an hour (In reply to comment #20) > > But it doesn't run forever... By "run forever", I mean the job has been created, but hasn't been removed from the job manager See https://bugs.eclipse.org/bugs/show_bug.cgi?id=198241 > it only runs very briefly every five minutes. ... if the workspace is small. I am using the following configuration: - Eclipse, WTP, BIRT, EGit, m2e, Mylyn, JBoss Tools ... - 100+ projects in the workspace - the "Include all plug-ins from target in Java Search" preference is checked. Eclipse takes a few minutes to save the workspace when exiting. JBoss Tools tests can create a big workspace. > When a job is scheduled to run five minutes in the future, the job manager > continues to be idle until that job runs. In your scenario it appears the > job manager is non-idle for an hour Right. The Job Manager is non-idle and the only job included in the job manager is the workspace save job. I believe other users will also have a problem with this job. (In reply to comment #21) > > ... if the workspace is small. This job has *nothing* to do with the workspace ... and the size of the workspace will not effect it at all. PW (In reply to comment #22) > (In reply to comment #21) > > > > ... if the workspace is small. > > > This job has *nothing* to do with the workspace ... and the size of the > workspace will not effect it at all. > > PW The job just persists/closes editors? If so, it is possible that we have a deadlock when some of our tests opens an editor and makes it dirty. Will check that. Thanks. See also bug 405542. (In reply to comment #23) > The job just persists/closes editors? If so, it is possible that we have a > deadlock when some of our tests opens an editor and makes it dirty. Will > check that. It won't close editors, but it will ask any persistable ones to update their persistent state. PW By the way, isn't this bug (at least point 1 of the description) related to #400334? *** Bug 400334 has been marked as a duplicate of this bug. *** This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |