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

Bug 358396

Summary: [ui] MarkOccurrenceJob can cause deadlock
Product: [Modeling] TMF Reporter: Knut Wannheden <knut.wannheden>
Component: XtextAssignee: Jan Koehnlein <jan>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: christian.dietrich.opensource, tmf.xtext-inbox
Version: 2.0.1Flags: jan: indigo+
Target Milestone: SR2   
Hardware: PC   
OS: Mac OS X - Carbon (unsup.)   
Whiteboard:
Attachments:
Description Flags
patch for OccurrenceMarker
none
updated patch
none
New patch sven.efftinge: iplog+

Description Knut Wannheden CLA 2011-09-21 07:29:51 EDT 
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
Comment 1 Knut Wannheden CLA 2011-09-21 08:12:47 EDT 
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.
Comment 2 Knut Wannheden CLA 2011-09-21 09:10:38 EDT 
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.
Comment 3 Knut Wannheden CLA 2011-09-21 09:29:17 EDT 
Created attachment 203761 [details]
updated patch

The attached patch modifies the OccurrenceMarker and the EditorResourceAccess classes as proposed.
Comment 4 Jan Koehnlein CLA 2011-09-26 11:00:45 EDT 
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.
Comment 5 Knut Wannheden CLA 2011-09-26 11:19:29 EDT 
(In reply to comment #4)
> I've modified the patch with regard to that. Please revise.

Looks good to me.
Comment 6 Jan Koehnlein CLA 2011-09-27 03:19:43 EDT 
Patch committed.
Comment 7 Jan Koehnlein CLA 2011-09-27 03:19:58 EDT 
Fixed.
Comment 8 Holger Schill CLA 2011-09-29 03:44:18 EDT 
*** Bug 353332 has been marked as a duplicate of this bug. ***
Comment 9 Karsten Thoms CLA 2017-09-19 17:18:08 EDT 
Closing all bugs that were set to RESOLVED before Neon.0
Comment 10 Karsten Thoms CLA 2017-09-19 17:29:28 EDT 
Closing all bugs that were set to RESOLVED before Neon.0