Bug 578871 - prevent UI freeze during UNDO operation + autobuild #6
Conversation
While JDT's refactoring correctly tries to interrupt autobuild the UNDO of the same operation did not. In general all UNDO Operations did not try to interrupt autobuild. This adds dependency from ui to resources to make the History operations become aware of resource scheduling rule.
|
As mentioned on Gerrit, I'm not really convinced that this a proper fix to the right problem. The actual problem is trying to begin a rule in the UI Thread should show some progress dialog. That would cover all cases of that kind without need for the operation to care about the progress report, and instead to focus on actually doing its job. |
|
How about submit this and just revert if it makes any trouble or when you found a better solution? |
|
OK, let's do that. |
|
I opened #8 about a general solution. |
| try { | ||
| try { | ||
| // interrupt autobuild or show Wait/Cancel Dialog: | ||
| Runnable r = () -> manager.beginRule(rule, null); |
There was a problem hiding this comment.
This change causes a regression. If the Undo task joins on a workspace rule, it hangs now.
So starting a rule here is not OK.
I would propose to revert the change.
There was a problem hiding this comment.
If the Undo task joins on a workspace rule
how to reproduce that?
There was a problem hiding this comment.
Have custom IUndoableOperation (we have one that extends AbstractOperation) that runs operation in a different thread and there takes the workspace rule.
@Override
public IStatus undo(IProgressMonitor monitor, IAdaptable info) throws ExecutionException {
IProgressService progressService = PlatformUI.getWorkbench().getProgressService();
try {
progressService.run(true, false, new IRunnableWithProgress() {
@Override
public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException {
ISchedulingRule rule = ResourcesPlugin.getWorkspace().getRoot();
Job.getJobManager().beginRule(rule, monitor); // here we hang now
// ... do something that needs workspace
}
});
} catch (InvocationTargetException | InterruptedException e) {
}
return Status.OK_STATUS;
}
There was a problem hiding this comment.
Basically, locking anything accessible to 3rd party code before starting undo is not OK, because "undo" in 3rd party code can require the same lock.
Also interesting (I haven't noticed that first), that the org.eclipse.ui.workbench bundle in question had no references to org.eclipse.core.resources bundle before, I think for a good reason.
While JDT's refactoring correctly tries to interrupt autobuild the UNDO
of the same operation did not. In general all UNDO Operations did not
try to interrupt autobuild.
This adds dependency from ui to resources to make the History operations
become aware of resource scheduling rule.
from https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/192458