eclipse-uml2-bot
commented
1 week ago By Ed Willink on Nov 23, 2023 04:52
(In reply to Eike Stepper from comment #0)
I'm starting to think that EMF is really generally not safe in multi-threaded applications. Using separate ResourceSets for different threads probably reduces the risk for actual problems, but never to 0%. And I'm sure that EPackage.Registry.INSTANCE and Resource.Factory.Registry.INSTANCE suffer the same problem.
EMF is not threadsafe. Full stop.
EMF is perhaps threadsafe if all global context is warmed up before a second thread gets going.
It is the application's responsibility to ensure that such warmup occurs.
This was fine for traditional EMF as a client 'library'.
However once 'live validation' was introduced, EMF became its own application; two threads both supporting org.eclipse.emf code. Some synchronization via the ResourceSet was introduced to mitigate problems. These problems were much worse where live validation of an OCL constraint could activate an OCL parser to populate a compiled constraint cache and everything that goes with it.
Further challenges occur with a concurrent builder/validator.
In this case, I suspect that a JUnit test that provokes a second thread is an invalid test.
(The latest OCL introduces a local validation registry to avoid concurrent fighting in the global validation registry.)
| --- | --- | | Bugzilla Link | 582660 | | Status | NEW | | Importance | P3 normal | | Reported | Nov 22, 2023 17:09 EDT | | Modified | Nov 23, 2023 04:52 EDT | | Version | 5.5.2 | | Reporter | Eike Stepper |
Description
Created attachment 289233\ Exceptions on the console
I ran the "UML2 UML Tests Standalone" test suite quite a few times now and I noticed one occasionally failing test case: Bug332057Test.testConcurrentCacheAdapterAccess()
The exceptions on the console are varying. See my attachment for a selection.
Behavior is based on commit 7601b2ccc36d75accccf14e9835dd5551e13df7d.
I tested only with the standalone suite (I think this is important, see below).
Bug #332057 and bug #335125 may be related.
As already stated in bug #332057, under debugger control the timing is affected such that the problem is no longer reproducible.
Here's an interesting part of one of the exceptions I saw:
Caused by: org.eclipse.emf.common.util.BasicEList$BasicIndexOutOfBoundsException: index=7, size=8\ at org.eclipse.emf.common.util.BasicEList.get(BasicEList.java:346)\ at org.eclipse.emf.ecore.resource.impl.URIMappingRegistryImpl.getURI(URIMappingRegistryImpl.java:109)\ at org.eclipse.emf.ecore.resource.impl.ExtensibleURIConverterImpl$1.delegatedGetURI(ExtensibleURIConverterImpl.java:473)\ at org.eclipse.emf.ecore.resource.impl.URIMappingRegistryImpl.getURI(URIMappingRegistryImpl.java:120)\ at org.eclipse.emf.ecore.resource.impl.URIMappingRegistryImpl$URIMapImpl.getURI(URIMappingRegistryImpl.java:162)\ at org.eclipse.emf.ecore.resource.impl.ExtensibleURIConverterImpl.normalize(ExtensibleURIConverterImpl.java:407)\ at org.eclipse.emf.ecore.resource.impl.ExtensibleURIConverterImpl.normalize(ExtensibleURIConverterImpl.java:446)\ at org.eclipse.uml2.common.util.CacheAdapter$InverseCrossReferencer.normalizeURI(CacheAdapter.java:84)
"IndexOutOfBoundsException: index=7, size=8" - This can only be caused by unsynchronized access of multiple threads. The "size" was increased after the argument check was executed but before the exception message was formed.
The other interesting part of that stack trace is "ExtensibleURIConverterImpl$1.delegatedGetURI()". Here the ResourceSet-local URIConverter delegates to some global EMF registry, i.e., URIMappingRegistryImpl.INSTANCE, which is not thread-safe.
I'm starting to think that EMF is really generally not safe in multi-threaded applications. Using separate ResourceSets for different threads probably reduces the risk for actual problems, but never to 0%. And I'm sure that EPackage.Registry.INSTANCE and Resource.Factory.Registry.INSTANCE suffer the same problem.
What makes me feel a bit better, is that modifications to these registries are normally rare and early. This also explains why we don't hear more often about the resulting problems.
In fact the test case Bug332057Test.testConcurrentCacheAdapterAccess() is especially amenable to this kind of problem, and probably even more especially when run in standalone mode. This is because of the code in Bug332057Test.createResourceSet():
if (StandaloneSupport.isStandalone()) {\ // init touches some global registries, which may not be accessed\ // concurrently by multiple threads, so be careful to avoid\ // concurrent modifications and indices out of bounds\ synchronized (this) {\ StandaloneSupport.init(result);\ }\ }
Interestingly there is a comment about multi-threading issues! But the "synchronized (this)" there is not sufficient to work around the problem. It only limits the write accesses to a single thread. The other test threads may still read from those registries in parallel, and fail.
:notepad_spiral: uml-test-exceptions.txt