Home » Modeling » Papyrus » Deadlock using TransactionalEditingDomain in Papyrus\CDO
Deadlock using TransactionalEditingDomain in Papyrus\CDO [message #1715220] |
Fri, 20 November 2015 10:56 |
Thorsten Schlathölter Messages: 312 Registered: February 2012 Location: Düsseldorf |
Senior Member |
|
|
Hi,
we are using Papyrus together with CDO and we encounter deadlock situations when we commit the transaction from a Dialog or Wizard context.
The following is roughly what happens:
We use the Eclipse LTK Refactoring Framework to perform model refactorings. Now the dialog opens, collects all changes and upon finish it startes to perform the individual changes which in our case changes the model. Finally the CDOTransaction is committed. We do this by calling save on the CDOAwareModelSet.
The Refactoring Wizards forks a process that performs the actual refactoring. This process locks the CDOTransaction. In order to have the UI responsive it polls the UIThread. Now when the commit has finished, a notification is fired which sets the modified state of the Resource to false. When this happens, the following method is called from TransactionChangeRecorder:
TransactionalEditingDomainImpl.broadcastUnbatched
Which sends the notification in an exclusive runnable.
Several Papyrus\GMF Objects are registered as listeners to these notifications and start update or notification Jobs (e.g. the ModelExplorer or DiagramEventBrokerThreadSafe) which themselve get a hold on the Display. These jobs are started from the Wizard polling the queue. Once they have been started, they try to get the Transaction which is still locked by the Wizard which itself is waiting for the UIJob to complete. In our case this happens always.
This is a typical stack of a thread which is deadlocked.
Object.wait(long) line: not available [native method]
Semaphore.acquire(long) line: 43
UISynchronizer.syncExec(Runnable) line: 164
Display.syncExec(Runnable) line: 4761
DiagramEventBrokerThreadSafe.resourceSetChanged(ResourceSetChangeEvent) line: 63
TransactionalEditingDomainImpl$2.run() line: 827
CDOAwareTransactionalEditingDomain(PapyrusROTransactionalEditingDomain).runExclusive(Runnable) line: 270
CDOAwareTransactionalEditingDomain(TransactionalEditingDomainImpl).broadcastUnbatched(Notification) line: 818
CDOAwareTransactionalEditingDomain.broadcastUnbatched(Notification) line: 142
CDOAwareTransactionalEditingDomain$1(TransactionChangeRecorder).appendNotification(Notification) line: 319
CDOAwareTransactionalEditingDomain$1.appendNotification(Notification) line: 52
CDOAwareTransactionalEditingDomain$1(TransactionChangeRecorder).processResourceNotification(Notification) line: 272
CDOAwareTransactionalEditingDomain$1(TransactionChangeRecorder).notifyChanged(Notification) line: 238
CDOAwareTransactionalEditingDomain$1(DawnTransactionChangeRecorder).notifyChanged(Notification) line: 44
CDOResourceImpl(BasicNotifierImpl).eNotify(Notification) line: 374
CDOResourceImpl.setModified(boolean) line: 575
CDOModificationTrackingAdapter$1.committedTransaction(CDOTransaction, CDOCommitContext) line: 45
CDOTransactionImpl$CDOCommitContextImpl.postCommit(CDOSessionProtocol$CommitTransactionResult) line: 3318
CDOSingleTransactionStrategyImpl.commit(InternalCDOTransaction, IProgressMonitor) line: 65
CDOTransactionImpl.commitSynced(IProgressMonitor) line: 1316
CDOTransactionImpl.commit(IProgressMonitor) line: 1290
CDOResourceImpl.save(Map<?,?>) line: 1332
CSSNotationModel(EMFLogicalModel).saveModel() line: 80
CdoResourceSet(ProjectResourceSet).save_takenFromModelSet(IProgressMonitor) line: 545
CdoResourceSet(ProjectResourceSet).save_takenFromCDOAwareModelSet(IProgressMonitor) line: 447
CdoResourceSet(ProjectResourceSet).save(IProgressMonitor) line: 401
ProjectTransactionManager$AutoCommitHandler.commandStackChanged(EventObject) line: 165
NotifyingWorkspaceCommandStack$2.historyNotification(OperationHistoryEvent) line: 142
DefaultOperationHistory$2.run() line: 933
SafeRunner.run(ISafeRunnable) line: 42
DefaultOperationHistory.notifyListeners(OperationHistoryEvent) line: 922
DefaultOperationHistory.notifyDone(IUndoableOperation) line: 988
DefaultOperationHistory.closeOperation(boolean, boolean, int) line: 1343
UndoManager2.changePerformed(Change, boolean) line: 153
PerformChangeOperation$1.run(IProgressMonitor) line: 265
Workspace.run(IWorkspaceRunnable, ISchedulingRule, int, IProgressMonitor) line: 2241
UIPerformChangeOperation(PerformChangeOperation).executeChange(IProgressMonitor) line: 306
UIPerformChangeOperation.executeChange(IProgressMonitor) line: 92
UIPerformChangeOperation(PerformChangeOperation).run(IProgressMonitor) line: 223
Workspace.run(IWorkspaceRunnable, ISchedulingRule, int, IProgressMonitor) line: 2241
WorkbenchRunnableAdapter.run(IProgressMonitor) line: 87
ModalContext$ModalContextThread.run() line: 119
If I do not fire the notification via broadcastUnbatched, everything is fine. But then I do not get the modified notification.
Any thoughts or workaround for this?
Regards,
Thorsten
|
|
|
Re: Deadlock using TransactionalEditingDomain in Papyrus\CDO [message #1715229 is a reply to message #1715220] |
Fri, 20 November 2015 11:53 |
Camille Letavernier Messages: 952 Registered: February 2011 |
Senior Member |
|
|
Hi Thorsten,
We have noticed similar issues recently. In general, I believe that a UI Sync Exec running from a transaction (Listener or Pre/Post-commit) is a very bad practice that should be avoided
The DiagramEventBrokerThreadSafe class seems highly suspicious in that case. This class is from GMF; I'm not sure if we (in Papyrus) explicitly enable it or if it enabled by default in GMF
I haven't looked at the details of this class yet, but here's the Javadoc. This might give hints as to how to avoid these deadlocks:
Quote: * This is an extension of the DiagramEventBroker that has special handling for notifications that
* occurs from a worker thread / non-UI thread. If the notification occurs on the main thread
* then execution is delegated to the super class immediately. If execution is not on the main thread
* then there are 2 scenarios that have to be considered.
*
* The first scenario is for a long operation
* that has been executed with a progress meter, where the progress meter is displaying UI on the main
* thread and there is a background thread that is being executed that the progress meter is monitoring.
* For this scenario, the UI updates on the diagram viewer have been disabled so as to avoid concurrency
* issues. When notifications are handled, they are synchronized to the main thread to avoid
* SWTExceptions when UI tries to access SWT resources when updating figures or other UI.
*
* The second scenario is for when a job has been executed on a worker thread, but has not been executed
* through the OperationHistory. Consequently, there is no hook for turning off display updates. This
* means that if the notifications were to continue on the worker thread, then the display could update
* at the same time on the main thread thereby causing concurrent modification errors and other errors.
* In this case, we need to resynchronize the notifications with the main thread. In order to do this
* it is necessary to run the notifications in an synchronous runnable that will run immediately
* on the main thread.
Regards,
Camille
Camille Letavernier
|
|
|
Re: Deadlock using TransactionalEditingDomain in Papyrus\CDO [message #1715234 is a reply to message #1715229] |
Fri, 20 November 2015 13:00 |
|
Hi, Camille, Thorsten,
Indeed, this event-broker in GMF had to resort to a syncExec on the
display. I don't remember much from more than a decade ago, but I do
recall that there was a lot of effort spent on looking for robust
alternatives to this known deadlock risk, and in the end there just
wasn't any.
A responsible application does want to do long-running work like
refactoring off of the UI thread, but as you have noticed there are two
locks involved in this case (one CDO and one GMF) that inevitably end
up deadlocking. I expect that a solution will require an extension of
the diagram event-broker that is CDO-aware, to properly handle the CDO
transaction lock. If and when you do find a solution, it would make a
good contribution to CDO's Dawn component, which I would be happy to
review.
Cheers,
Christian
On 2015-11-20 11:54:00 +0000, Camille Letavernier said:
> Hi Thorsten,
>
> We have noticed similar issues recently. In general, I believe that a
> UI Sync Exec running from a transaction (Listener or Pre/Post-commit)
> is a very bad practice that should be avoided
>
> The DiagramEventBrokerThreadSafe class seems highly suspicious in that
> case. This class is from GMF; I'm not sure if we (in Papyrus)
> explicitly enable it or if it enabled by default in GMF
>
> I haven't looked at the details of this class yet, but here's the
> Javadoc. This might give hints as to how to avoid these deadlocks:
>
> Quote:
>> * This is an extension of the DiagramEventBroker that has special
>> handling for notifications that
>> * occurs from a worker thread / non-UI thread. If the notification
>> occurs on the main thread
>> * then execution is delegated to the super class immediately. If
>> execution is not on the main thread
>> * then there are 2 scenarios that have to be considered. * * The first
>> scenario is for a long operation
>> * that has been executed with a progress meter, where the progress
>> meter is displaying UI on the main
>> * thread and there is a background thread that is being executed that
>> the progress meter is monitoring.
>> * For this scenario, the UI updates on the diagram viewer have been
>> disabled so as to avoid concurrency
>> * issues. When notifications are handled, they are synchronized to the
>> main thread to avoid
>> * SWTExceptions when UI tries to access SWT resources when updating
>> figures or other UI.
>> * * The second scenario is for when a job has been executed on a worker
>> thread, but has not been executed
>> * through the OperationHistory. Consequently, there is no hook for
>> turning off display updates. This
>> * means that if the notifications were to continue on the worker
>> thread, then the display could update
>> * at the same time on the main thread thereby causing concurrent
>> modification errors and other errors.
>> * In this case, we need to resynchronize the notifications with the
>> main thread. In order to do this
>> * it is necessary to run the notifications in an synchronous runnable
>> that will run immediately
>> * on the main thread.
>
>
> Regards,
> Camille
|
|
|
Goto Forum:
Current Time: Thu Mar 28 12:23:49 GMT 2024
Powered by FUDForum. Page generated in 0.02420 seconds
|