Community
Participate
Working Groups
The implementation of the MarkOccurrenceJob can result in a deadlock (including the UI thread!) in the following unfortunate situation: 1. The MarkOccurrenceJob is triggered and through DefaultOccurrenceComputer#createAnnotationMap() successfully obtains a read-only document lock (RO(DOC)). 2. In this transaction it continues by synchronously executing a runnable on the UI thread (see EditorResourceAccess#readOnly()). 3. Meanwhile the Xtext reconciler requests a RW(DOC) lock but has to wait until the MarkOccurrenceJob releases its RO(DOC) lock. 4. Now the UI thread starts running the runnable from step 2 and also requests an RO(DOC) lock but is blocked because the reconciler should first get its chance to run. I was able to simulate this with some carefully placed breakpoints. See stack traces below. I think the MarkOccurrenceJob should be changed to act more like JDT's SemanticHighlightingReconciler (see method scheduleJob()): setSystem(true), setPriority(Job.DECORATE), and by first having the job to wait for the previously scheduled job to join(). This of course doesn't solve the issue. So additionally I think the UI synchronization should be removed from EditorResourceAccess. If that isn't possible (is it?) we have to think of a different solution. MarkOccurrenceJob stack trace: Thread [Worker-7] (Suspended) Object.wait(long) line: not available [native method] [local variables unavailable] Semaphore.acquire(long) line: 43 UISynchronizer.syncExec(Runnable) line: 168 Display.syncExec(Runnable) line: 3945 EditorResourceAccess$1(DisplayRunnableWithResult<T>).syncExec() line: 22 EditorResourceAccess.readOnly(URI, IUnitOfWork<R,ResourceSet>) line: 74 DefaultReferenceFinder.findLocalReferences(IQueryData, ILocalResourceAccess, IAcceptor<IReferenceDescription>, IProgressMonitor) line: 90 DefaultOccurrenceComputer$1.exec(XtextResource) line: 96 DefaultOccurrenceComputer$1.exec(Object) line: 1 XtextDocument$XtextDocumentLocker(AbstractReadWriteAcces<P>).readOnly(IUnitOfWork<T,P>) line: 32 XtextDocument.readOnly(IUnitOfWork<T,XtextResource>) line: 79 DefaultOccurrenceComputer.createAnnotationMap(XtextEditor, ITextSelection, SubMonitor) line: 82 OccurrenceMarker$MarkOccurrenceJob.run(IProgressMonitor) line: 119 Worker.run() line: 55 Reconciler stack trace: Thread [Worker-0] (Stepping) Unsafe.park(boolean, long) line: not available [native method] [local variables unavailable] LockSupport.park(Object) line: 158 ReentrantReadWriteLock$NonfairSync(AbstractQueuedSynchronizer).parkAndCheckInterrupt() line: 811 ReentrantReadWriteLock$NonfairSync(AbstractQueuedSynchronizer).acquireQueued(AbstractQueuedSynchronizer$Node, int) line: 842 ReentrantReadWriteLock$NonfairSync(AbstractQueuedSynchronizer).acquire(int) line: 1178 ReentrantReadWriteLock$WriteLock.lock() line: 807 [local variables unavailable] XtextDocument$XtextDocumentLocker(AbstractReadWriteAcces<P>).modify(IUnitOfWork<T,P>) line: 45 XtextDocument$XtextDocumentLocker.modify(IUnitOfWork<T,XtextResource>) line: 182 XtextDocument.internalModify(IUnitOfWork<T,XtextResource>) line: 91 XtextDocumentReconcileStrategy.reconcile(IRegion) line: 33 XtextReconciler.run(IProgressMonitor) line: 239 Worker.run() line: 55 UI thread stack trace: Daemon Thread [Thread-1] (Stepping) Unsafe.park(boolean, long) line: not available [native method] [local variables unavailable] LockSupport.park(Object) line: 158 ReentrantReadWriteLock$NonfairSync(AbstractQueuedSynchronizer).parkAndCheckInterrupt() line: 811 ReentrantReadWriteLock$NonfairSync(AbstractQueuedSynchronizer).doAcquireShared(int) line: 941 ReentrantReadWriteLock$NonfairSync(AbstractQueuedSynchronizer).acquireShared(int) line: 1261 ReentrantReadWriteLock$ReadLock.lock() line: 594 [local variables unavailable] XtextDocument$XtextDocumentLocker(AbstractReadWriteAcces<P>).readOnly(IUnitOfWork<T,P>) line: 28 XtextDocument.readOnly(IUnitOfWork<T,XtextResource>) line: 79 EditorResourceAccess$1.run() line: 63 DisplayRunnableWithResult$1.run() line: 25 UILockListener.doPendingWork() line: 155 UISynchronizer$3.run() line: 158 RunnableLock.run() line: 35 UISynchronizer(Synchronizer).runAsyncMessages(boolean) line: 134 Display.runAsyncMessages(boolean) line: 3405 Display.readAndDispatch() line: 3102 Workbench.runEventLoop(Window$IExceptionHandler, Display) line: 2405 Workbench.runUI() line: 2369 Workbench.access$4(Workbench) line: 2221 Workbench$5.run() line: 500 Realm.runWithDefault(Realm, Runnable) line: 332 Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 493 PlatformUI.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 149 IDEApplication.start(IApplicationContext) line: 113 EclipseAppHandle.run(Object) line: 194 EclipseAppLauncher.runApplication(Object) line: 110 EclipseAppLauncher.start(Object) line: 79 EclipseStarter.run(Object) line: 368 EclipseStarter.run(String[], Runnable) line: 179 NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method] NativeMethodAccessorImpl.invoke(Object, Object[]) line: 39 DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 25 Method.invoke(Object, Object...) line: 597 Main.invokeFramework(String[], URL[]) line: 559 Main.basicRun(String[]) line: 514 Main.run(String[]) line: 1311 Main.main(String[]) line: 1287
Created attachment 203754 [details] patch for OccurrenceMarker Actually the JobManager automatically takes care of rescheduling an already running job, so the only things I suggest changing is setting it as a system job and using the DECORATE priority.
Apparently the EditorResourceAccess at least requires the UI thread to call IWorkbench#getActiveWorkbenchWindow() in #getOpenDocument(). So if that's how the current open editor must be obtained, then I suppose it should be possible to wrap just that part into a DisplayRunnableWithResult. At least that seems to work for the OccurrenceMarker and should not result in any deadlocks.
Created attachment 203761 [details] updated patch The attached patch modifies the OccurrenceMarker and the EditorResourceAccess classes as proposed.
Created attachment 204015 [details] New patch Thanks for the patch. We need to be on the UI tread to access the active workbench page, and thereafter the editor and its document if it is opened. I guess it is a good idea to hold the UI lock until the document is determined to avoid problems with closing editors. It should be safe to release it for the actual IUnitOfWork. I've modified the patch with regard to that. Please revise.
(In reply to comment #4) > I've modified the patch with regard to that. Please revise. Looks good to me.
Patch committed.
Fixed.
*** Bug 353332 has been marked as a duplicate of this bug. ***
Closing all bugs that were set to RESOLVED before Neon.0
