Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 346715 - [EMF Runtime Extensions] IMetaModelDescriptor methods of MetaModelDescriptorRegistry taking EObject or Resource arguments should not start new EMF transactions
Summary: [EMF Runtime Extensions] IMetaModelDescriptor methods of MetaModelDescriptorR...
Status: CLOSED FIXED
Alias: None
Product: Sphinx
Classification: Automotive
Component: Core (show other bugs)
Version: 0.7.0   Edit
Hardware: All All
: P3 critical (vote)
Target Milestone: 0.7.0   Edit
Assignee: Stephan Eberle CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-20 11:27 EDT by Stephan Eberle CLA
Modified: 2021-07-14 02:14 EDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Eberle CLA 2011-05-20 11:27:57 EDT
= Problem description =

Currently, MetaModelDescriptorRegistry methods enabling the IMetaModelDescriptor for an EObject or EMF Resource to be retrieved are implemented in a thread safe way, i.e., they internally create EMF read transactions before evaluating the EObject of Resource in question. However, this is not quite appropriate because in most cases such read transaction, if required, are already created by the client code that calls the MetaModelDescriptorRegistry methods. 

In the best case, the fact of creating nested read transactions entails some performance overhead. But under certain conditions the IMetaModelDescriptor retrieval methods are called in moments where nested transaction even must not be created. Otherwise EMF transaction somehow falls completely out of pace and produces exceptions.

For example, when a new read transaction is created while an outer or previous write transaction is committed and consolidates its changes like this:

		TransactionChangeRecorder(BasicChangeRecorder).setChangeDescription(ChangeDescription) line: 67	
		TransactionChangeRecorder(ChangeRecorder).beginRecording(ChangeDescription, Collection<?>) line: 144	
		TransactionChangeRecorder(ChangeRecorder).beginRecording(Collection<?>) line: 118	
		TransactionChangeRecorder.beginRecording() line: 104	
		EMFOperationTransaction(TransactionImpl).startRecording() line: 637	
		EMFOperationTransaction(TransactionImpl).resume(TransactionChangeDescription) line: 689	
		TransactionImpl.close() line: 724	
		TransactionImpl.commit() line: 474	
		ExtendedWorkspaceEditingDomainFactory$ExtendedWorkspaceEditingDomain(TransactionalEditingDomainImpl).runExclusive(Runnable) line: 333	
		TransactionUtil.runExclusive(TransactionalEditingDomain, RunnableWithResult<? extends T>) line: 328	
		EcorePlatformUtil.getFile(Resource) line: 519	
		MetaModelDescriptorRegistry.getOldDescriptor(Resource) line: 769	
		MetaModelDescriptorRegistry.getDescriptor(EObject) line: 801	
		MetaModelDescriptorRegistry.getDescriptor(Resource) line: 544	
		ModelDescriptorRegistry.internalGetModel(Resource) line: 302	
		ModelDescriptorRegistry.getModel(Resource) line: 292	
		ScopingResourceSetImpl.getResourceScopes(Object, Resource) line: 172	
		ScopingResourceSetImpl.getResourcesInScope(Object, boolean, boolean) line: 98	
		ScopingResourceSetImpl.getResourcesInScope(Object) line: 76	
		ScopingResourceSetImpl.getResourcesToSearchIn(URI, boolean, EObject) line: 339	
		ScopingResourceSetImpl.getEObjectInScope(URI, boolean, EObject) line: 230	
		ConnectionImpl(ExtendedEObjectImpl).eResolveProxyInResourceSet(ResourceSet, InternalEObject) line: 94	
		ConnectionImpl(ExtendedEObjectImpl).eResolveProxy(InternalEObject) line: 50	
		ConnectionImpl.getTargetComponent() line: 171	
		ConnectionImpl.eGet(int, boolean, boolean) line: 287	
		ConnectionImpl(BasicEObjectImpl).eGet(EStructuralFeature, boolean, boolean) line: 1021	
		ConnectionImpl(BasicEObjectImpl).eGet(EStructuralFeature, boolean) line: 1013	
		ConnectionImpl(BasicEObjectImpl).eGet(EStructuralFeature) line: 1008	
		TransactionChangeRecorder(BasicChangeRecorder).eliminateEmptyChanges() line: 153	
		TransactionChangeRecorder(BasicChangeRecorder).consolidateChanges() line: 132	
		TransactionChangeRecorder(ChangeRecorder).consolidateChanges() line: 224	
		TransactionChangeRecorder(BasicChangeRecorder).endRecording() line: 106	
		TransactionChangeRecorder.endRecording() line: 119	
		EMFOperationTransaction(TransactionImpl).stopRecording() line: 660	
		EMFOperationTransaction(TransactionImpl).commit() line: 467	
		ExtendedWorkspaceEditingDomainFactory$1$1(AbstractEMFOperation).execute(IProgressMonitor, IAdaptable) line: 155	
		DefaultOperationHistory.execute(IUndoableOperation, IProgressMonitor, IAdaptable) line: 511	
		ExtendedWorkspaceEditingDomainFactory$1.doExecute(Command, Map<?,?>) line: 253	
		ExtendedWorkspaceEditingDomainFactory$1(AbstractTransactionalCommandStack).execute(Command, Map<?,?>) line: 165	
		ExtendedWorkspaceEditingDomainFactory$1.execute(Command) line: 207	
		ExtendedItemPropertyDescriptor(ItemPropertyDescriptor).setPropertyValue(Object, Object) line: 1441	
		
then the outer/previous write transaction ends up loosing its change description and runs into the following NullPointerException (maby en EMF Transaction bug?):
	
java.lang.NullPointerException
	at org.eclipse.emf.ecore.change.util.ChangeRecorder.consolidateChanges(ChangeRecorder.java:204)
	at org.eclipse.emf.ecore.change.util.BasicChangeRecorder.endRecording(BasicChangeRecorder.java:106)
	at org.eclipse.emf.transaction.impl.TransactionChangeRecorder.endRecording(TransactionChangeRecorder.java:119)
	at org.eclipse.emf.transaction.impl.TransactionImpl.stopRecording(TransactionImpl.java:660)
	at org.eclipse.emf.transaction.impl.TransactionImpl.doRollback(TransactionImpl.java:567)
	at org.eclipse.emf.transaction.impl.TransactionImpl.rollback(TransactionImpl.java:547)
	at org.eclipse.emf.workspace.AbstractEMFOperation.rollback(AbstractEMFOperation.java:630)
	at org.eclipse.emf.workspace.AbstractEMFOperation.execute(AbstractEMFOperation.java:187)
	at org.eclipse.core.commands.operations.DefaultOperationHistory.execute(DefaultOperationHistory.java:511)
	at org.eclipse.sphinx.emf.workspace.domain.factory.ExtendedWorkspaceEditingDomainFactory$1.doExecute(ExtendedWorkspaceEditingDomainFactory.java:253)
	at org.eclipse.emf.transaction.impl.AbstractTransactionalCommandStack.execute(AbstractTransactionalCommandStack.java:165)
	at org.eclipse.sphinx.emf.workspace.domain.factory.ExtendedWorkspaceEditingDomainFactory$1.execute(ExtendedWorkspaceEditingDomainFactory.java:207)
	at org.eclipse.emf.edit.provider.ItemPropertyDescriptor.setPropertyValue(ItemPropertyDescriptor.java:1441)
	at org.eclipse.emf.edit.ui.provider.PropertySource.setPropertyValue(PropertySource.java:116)
	at org.eclipse.emf.transaction.ui.provider.TransactionalPropertySource.setPropertyValue(TransactionalPropertySource.java:165)
	at org.eclipse.emf.transaction.ui.provider.TransactionalPropertySource.setPropertyValue(TransactionalPropertySource.java:165)
	at org.eclipse.sphinx.emf.ui.properties.FilteringPropertySource.setPropertyValue(FilteringPropertySource.java:67)
	at org.eclipse.ui.views.properties.PropertySheetEntry.valueChanged(PropertySheetEntry.java:782)
	at org.eclipse.ui.views.properties.PropertySheetEntry.setValue(PropertySheetEntry.java:723)
	at org.eclipse.ui.views.properties.PropertySheetEntry.applyEditorValue(PropertySheetEntry.java:146)
	at org.eclipse.ui.views.properties.PropertySheetEntry$1.applyEditorValue(PropertySheetEntry.java:103)

= Resolution proposal =

* Skip thread-safety of MetaModelDescriptorRegistry methods for retrieving IMetaModelDescriptor from EObject or Resource and make sure that they neither create any read transactions internally nor call any code that does so (e.g, EcorePlatformUtil.getFile(Resource)).
* Introduce thread-safety in all client code that retrieves EObjects or Resources and directly or indirectly calls IMetaModelDescriptor retrieval methods for them by wrapping it in read transactions at the most possible outside level in the call hierarchy.
Comment 1 Stephan Eberle CLA 2011-05-23 12:26:06 EDT
Fixed as proposed in description.

Improved implementation of ProxyURICellEditor and ProxyURIFeatureEditorDialog along the way.
Comment 2 Balazs Grill CLA 2021-07-14 02:14:37 EDT
Mass-closing Resolved tickets