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

Bug 405356

Summary: [Workbench] Auto-save not re-enabled when preference set from 0 to X
Product: [Eclipse Project] Platform Reporter: Snjezana Peco <snjezana.peco>
Component: IDEAssignee: 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 Flags
A patch none

Description Snjezana Peco CLA 2013-04-10 09:52:18 EDT
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).

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. See https://github.com/jbosstools/jbosstools-base/blob/master/tests/plugins/org.jboss.tools.tests/src/org/jboss/tools/test/util/JobUtils.java and https://issues.jboss.org/browse/JBIDE-13973

I suppose this has to be run in a separate thread that is always created, sleeps for saveInterval ms (or some constant interval if saveInterval=0) and, after that, calls persist in a new job if saveInterval>0.

Also, I suppose this preference would have to be 0 by default because that is behaviour users expect. This would also speed up tests that don't test this preference.

I am willing to create a patch if you want.
Comment 1 Dani Megert CLA 2013-04-10 10:07:04 EDT
> 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!
Comment 2 Paul Webster CLA 2013-04-10 10:52:12 EDT
(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
Comment 3 Snjezana Peco CLA 2013-04-10 11:36:23 EDT
Created attachment 229570 [details]
A patch

The patch fixes all described issues.
Comment 4 Snjezana Peco CLA 2013-04-10 11:42:43 EDT
(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.
Comment 5 Paul Webster CLA 2013-04-10 11:43:12 EDT
(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
Comment 6 Paul Webster CLA 2013-04-10 11:46:03 EDT
(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
Comment 7 Snjezana Peco CLA 2013-04-10 12:07:37 EDT
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.
Comment 8 Paul Webster CLA 2013-04-10 12:16:10 EDT
(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
Comment 9 Snjezana Peco CLA 2013-04-10 12:56:26 EDT
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
Comment 10 Paul Webster CLA 2013-04-10 13:10:11 EDT
(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
Comment 11 Snjezana Peco CLA 2013-04-10 16:10:46 EDT
> 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.
Comment 12 Max Rydahl Andersen CLA 2013-04-11 05:48:09 EDT
+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.
Comment 13 Paul Webster CLA 2013-04-11 07:31:07 EDT
I've opened bug 405456 to allow tests to disable the auto-save job

PW
Comment 14 John Arthorne CLA 2013-04-11 09:22:44 EDT
(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.
Comment 15 Snjezana Peco CLA 2013-04-11 11:00:45 EDT
(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.
Comment 16 John Arthorne CLA 2013-04-11 12:01:22 EDT
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.
Comment 17 Snjezana Peco CLA 2013-04-11 12:01:51 EDT
The problem isn't what the autoSaveJob job works, but that it runs forever.
#405456 solves it.
Comment 18 Snjezana Peco CLA 2013-04-11 13:30:13 EDT
(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.
Comment 19 Snjezana Peco CLA 2013-04-11 13:33:08 EDT
> Can you debug your test ...

I can't debug the tests because the failure happens randomly on Hudson.
It isn't reproducible locally.
Comment 20 John Arthorne CLA 2013-04-11 14:36:03 EDT
(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
Comment 21 Snjezana Peco CLA 2013-04-11 17:26:25 EDT
(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.
Comment 22 Paul Webster CLA 2013-04-11 17:46:28 EDT
(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
Comment 23 Snjezana Peco CLA 2013-04-11 18:03:48 EDT
(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.
Comment 24 Dani Megert CLA 2013-04-12 03:54:29 EDT
See also bug 405542.
Comment 25 Paul Webster CLA 2013-04-12 08:27:35 EDT
(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
Comment 26 Mauro Molinari CLA 2013-05-24 12:04:45 EDT
By the way, isn't this bug (at least point 1 of the description) related to #400334?
Comment 27 John Arthorne CLA 2013-05-24 16:07:17 EDT
*** Bug 400334 has been marked as a duplicate of this bug. ***
Comment 28 Eclipse Genie CLA 2019-12-26 15:37:44 EST
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.