Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various exceptions logged in Bug332057Test.testConcurrentCacheAdapterAccess() #118

Open
eclipse-uml2-bot opened this issue Nov 14, 2024 · 1 comment

Comments

@eclipse-uml2-bot
Copy link

| --- | --- |
| 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 7601b2c.

  • 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

@eclipse-uml2-bot
Copy link
Author

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant