Community
Participate
Working Groups
We provide a Resource Move refactoring participant for resources managed by our plug-in. The following line of code in the participant causes the deadlock: new MoveFilesAndFoldersOperation(<the active shell>).copyResources(<some resources>, <destination container>); The MoveFilesAndFoldersOperation class is from the "org.eclipse.ui.actions" package.
Can you please provide a VM dump. Without it its hard to tell what's going on. Using MoveFilesAndFoldersOperation inside a refactoring participant is very likely asking for trouble. The operation is written for the UI (it takes a shell) and interacts with the users. Refactoring participants act "headless". Changes created by a refactoring participant can be executed in any thread. There is no guarantee that they are executed in the UI thread (the refactoring core infrastructure doesn't know about the UI thread).
Sorry, since this is a public forum I cannot provide the stack trace from the proprietary product without authorization. If it can help I can give you the parts of the stack trace from the top of the main thread that involves just the Eclipse code and the 2 ModalContext threads (those threads seem to be in the deadlock): Thread [main] (Suspended) owns: org.eclipse.swt.widgets.RunnableLock (id=34727764) org.eclipse.swt.internal.win32.OS.WaitMessage() line: not available [native method] org.eclipse.swt.widgets.Display.sleep() line: 3375 org.eclipse.jface.operation.ModalContext$ModalContextThread.block() line: 154 org.eclipse.jface.operation.ModalContext.run (org.eclipse.jface.operation.IRunnableWithProgress, boolean, org.eclipse.core.runtime.IProgressMonitor, org.eclipse.swt.widgets.Display) line: 303 org.eclipse.ui.internal.progress.ProgressMonitorJobsDialog (org.eclipse.jface.dialogs.ProgressMonitorDialog).run(boolean, boolean, org.eclipse.jface.operation.IRunnableWithProgress) line: 447 org.eclipse.ui.internal.progress.ProgressMonitorJobsDialog.run (boolean, boolean, org.eclipse.jface.operation.IRunnableWithProgress) line: 261 org.eclipse.ui.actions.MoveFilesAndFoldersOperation (org.eclipse.ui.actions.CopyFilesAndFoldersOperation).copyResources (org.eclipse.core.resources.IResource[], org.eclipse.core.resources.IContainer) line: 473 Thread [ModalContext] (Suspended) owns: org.eclipse.swt.widgets.RunnableLock (id=34727764) waiting for: org.eclipse.swt.widgets.RunnableLock (id=34727764) org.eclipse.swt.widgets.RunnableLock(java.lang.Object).wait(long, int) line: not available [native method] org.eclipse.swt.widgets.RunnableLock(java.lang.Object).wait() line: 199 org.eclipse.ui.internal.UISynchronizer (org.eclipse.swt.widgets.Synchronizer).syncExec(java.lang.Runnable) line: 164 org.eclipse.ui.internal.UISynchronizer.syncExec(java.lang.Runnable) line: 28 org.eclipse.swt.widgets.Display.syncExec(java.lang.Runnable) line: 3401 org.eclipse.ltk.internal.ui.refactoring.UIPerformChangeOperation.execut eChange(org.eclipse.core.runtime.IProgressMonitor) line: 85 org.eclipse.ltk.internal.ui.refactoring.UIPerformChangeOperation (org.eclipse.ltk.core.refactoring.PerformChangeOperation).run (org.eclipse.core.runtime.IProgressMonitor) line: 192 org.eclipse.core.internal.resources.Workspace.run (org.eclipse.core.resources.IWorkspaceRunnable, org.eclipse.core.runtime.jobs.ISchedulingRule, int, org.eclipse.core.runtime.IProgressMonitor) line: 1719 org.eclipse.ltk.internal.ui.refactoring.WorkbenchRunnableAdapter.run (org.eclipse.core.runtime.IProgressMonitor) line: 86 org.eclipse.jface.operation.ModalContext$ModalContextThread.run() line: 113 Thread [ModalContext] (Suspended) owns: org.eclipse.core.internal.jobs.ThreadJob (id=34727724) owns: org.eclipse.ui.actions.CopyFilesAndFoldersOperation$2 (id=34727732) org.eclipse.core.internal.jobs.ThreadJob(java.lang.Object).wait(long, int) line: not available [native method] org.eclipse.core.internal.jobs.ThreadJob(java.lang.Object).wait(long) line: 231 org.eclipse.core.internal.jobs.ThreadJob.joinRun (org.eclipse.core.runtime.IProgressMonitor) line: 170 org.eclipse.core.internal.jobs.ImplicitJobs.begin (org.eclipse.core.runtime.jobs.ISchedulingRule, org.eclipse.core.runtime.IProgressMonitor, boolean) line: 88 org.eclipse.core.internal.jobs.JobManager.beginRule (org.eclipse.core.runtime.jobs.ISchedulingRule, org.eclipse.core.runtime.IProgressMonitor) line: 190 org.eclipse.core.internal.resources.WorkManager.checkIn (org.eclipse.core.runtime.jobs.ISchedulingRule, org.eclipse.core.runtime.IProgressMonitor) line: 96 org.eclipse.core.internal.resources.Workspace.prepareOperation (org.eclipse.core.runtime.jobs.ISchedulingRule, org.eclipse.core.runtime.IProgressMonitor) line: 1674 org.eclipse.core.internal.resources.Workspace.run (org.eclipse.core.resources.IWorkspaceRunnable, org.eclipse.core.runtime.jobs.ISchedulingRule, int, org.eclipse.core.runtime.IProgressMonitor) line: 1714 org.eclipse.ui.actions.CopyFilesAndFoldersOperation$2 (org.eclipse.ui.actions.WorkspaceModifyOperation).run (org.eclipse.core.runtime.IProgressMonitor) line: 110 org.eclipse.jface.operation.ModalContext$ModalContextThread.run() line: 113 Hope this helps.
I looked at the stack trace and here is what's happening: - refactoring change execution starts. This will look the workspace. - parts of the change execution takes place in the UI thread. Refactoring transfers the workspace lock for this purpose into the UI thread. - the participant starts another modal context thread from the UI thread by calling the MoveFilesAndFoldersOperation. The deadlock occurs due to the fact that the workspace lock is held by the UI thread and that the MoveFilesAndFoldersOperation in the modal context thread is trying to aquire the workspace lock as well. The only solution I see is that the MoveFilesAndFoldersOperation operation transfers the locks already owned by the UI thread to the modal context thread using the IThreadListener API. Please note that this deadlock scenario isn't refactoring specific. As pointed out in bug 105491 this can easily be produced by locking the workspace in the UI thread and then executing the MoveFilesAndFoldersOperation. There is little refactoring or JDT/UI can do to fix this. Moving to Platform/UI to consider transfering rules in operations before starting the modal context thread. However before doing so we should agree on if this is the path we want to follow. May be we should discuss this topic better in bug 105491.
Is there any news on this? I have not seen any Bugzilla activity on this bug (nor the 105491) in a while and was wondering what the plans are since it is really important for us.
John has checked this out and it is actually a dup of Bug 105491. Please continue discussion there. *** This bug has been marked as a duplicate of 105491 ***
I'm actually going to reopen because this case is subtely different.
Created attachment 28128 [details] Test case This test case reproduces the deadlock
Created attachment 28129 [details] Patch to WorkspaceModifyOperation This patch addresses the problem by throwing an exception in cases that would otherwise deadlock.
Created attachment 28131 [details] Patch to WorkspaceModifyOperation Sorry, that patch was garbled because I applied core team formatter settings to the file. This new patch does not alter the formatting.
Throwing the exception is a good step forward for the bug 105491. It does not solve this problem though. Two public Eclipse APIs (refactoring framework and MoveFilesAndFoldersOperation class) go into a deadlock and instead now will fail with a thrown exception. That's admittedly better but the real fix is to have those two things work together or to document (through javadoc and/or declared exceptions) what their limitations are. The current javadoc on MoveFilesAndFoldersOperation is following: "Moves files and folders. This class may be instantiated; it is not intended to be subclassed.".
Agreed - what we need is to know the specific places where you are having problems so that we can update the javadoc. I'll certainly update MoveFilesAndFoldersOperation with this. But be aware that different components are owned by different people so we'll need to know about them in seperatoe bug reports to be sure they son't get missed.
I was kind of hoping that you'd opt for fixing them to work together rather than documenting that they do not :) The problem is we cannot hunt down all of those places. We'll report the ones that we hit in our existing code but judging by the discussions on these two bug reports there will be numerous others through the Eclipse.
After talking to John what I think we want to do is remove your need to spin the event loop to use CopyFilesAndFoldersOperation - this is an example of code written before the jobs framework that should move over. See Bug 112252. I think we should apply this patch still in order to prevent the deadlock but it isn't a suffecient fix until we implement Bug 112251.
P.S. We'll apply the patch once we hear from Dirk.
Just to follow on Tod's comment: the strange thing about Copy/MoveFilesAndFoldersOperation is that it is not really an operation - its API must be called from the UI thread. Thus, if someone is already running an operation and wants to call it, they need to async into the UI to call it. I haven't seen all the code involved in this case, but if there was an equivalent of MoveFilesAndFoldersOperation.copyResources that could be called from outside the UI thread I think it would prevent this case of two modal context operations happening at once.
I agree with both comments :) The problem on the client code side is that it is a refactoring participant - it has no influence over whether it is going to run in a UI thread or not. Of course, refactoring runs on the UI thread but that cannot be changed by the participant. The idea of removing the dependency on the UI sounds like a right thing to do.
Tod, I am not sure what I should comment on, so here are some general observations/questions: - why doesn't WorkspaceModifyOperation transfer the rule to the modal context thread. The reason that we spin the event loop is to allow cancelation of the operation. This allows async execs to be executed but those should never rely on the fact that the UI thread owns a lock when they get executed. - the thread in which refactorings are executed isn't specified and depends on the implementor of the refactoring (ltk.core doesn't know anything about UI and therefore can't spec the thread). So participants should never rely on the fact that they are executed in the UI thread. They can be executed in some other thread as well if the refactoring decides to do so. - I agree that removing the dependency on the UI for the operations sounds like a good thing to do This is a different topic but we might want to think about adding knowledge of scheduling rules to Display#sync/async exec as well. This would allow to queue all async exec if the require a lock that is currently held by another thread. Additionally we could throw an exception if an sync exec is executed which results in a dead lock.
Comment on WorkspaceModifyOperation transferring the rule: I'm concerned about cases where the UI thread owns some scheduling rule that the modal context operation does not know about. If we transferred that rule, and it conflicts with the rule that the modal context operation, then an exception will occur in the modal context anyway. Transferring the rule would only suceed in the case where the modal context thread will acquire the same rule that was owned by the UI thread (or a nested rule as specified by ISchedulingRule#contains). Comment on rules with asyncExec: This is exactly the purpose of UIJob: UIJob = Display.asyncExec + a scheduling rule. I guess the synchronous equivalent is IProgressService.runInUI(IRunnableContext, IRunnableWithProgress, ISchedulingRule).
Thanks for the UIJob hint. Wasn't aware of this.
I have released the patch to WorkspaceModifyOperation. This is a good first step - the deadlock case is detected and an exception is thrown instead. Seperately, the MoveFilesAndFoldersOperation should be replaced with an operation that can be run outside the UI thread (bug 112251). Added test org.eclipse.ui.tests.concurrency.TestBug108162 to the UI automated test suites.
Verified (code is released, automated test suite is running as part of the builds)
