Open eclipse-qvt-oml-bot opened 5 days ago
By Ed Willink on Oct 11, 2017 06:22
(In reply to Ed Willink from comment #0)
From the stack trace the removal of entries from CollectionTypeImpl.oclOperations looks very very smelly.
This is in Ecore OCL. It is actually working on a 'temporary' that is set to all operations then trimmed by removal of iterators.
Examining
org.eclipse.emf.common.util.BasicEList$BasicIndexOutOfBoundsException: index=47, size=37\ at org.eclipse.emf.common.util.BasicEList.remove(BasicEList.java:601)\ at org.eclipse.emf.common.util.AbstractEList.removeAll(AbstractEList.java:486)
the BasicIndexOutOfBoundsException is impossible (in the absence of concurrency.) Opening out my debugger trace I find the\ filletted traces below showing concurrent calls to doFindCollidingOperation
Clearly there is an initialization hazard between the main/UI thread and the QvtReconciler.
org.eclipse.m2m.internal.tests.qvt.oml.ui.AllTests [JUnit Plug-in Test] \
org.eclipse.equinox.launcher.Main at localhost:55137 \
Thread [main] (Suspended) \
owns: RunnableLock (id=2303) \
BasicEList$FastCompare
By Ed Willink on Oct 11, 2017 06:43
(In reply to Ed Willink from comment #1)
Clearly there is an initialization hazard between the main/UI thread and the QvtReconciler.
With a bit of luck synchronizing QvtEnvironmentBase.doFindCollidingOperation may make the problem go away.
Except that I see intermittent failures on:
org.eclipse.m2m.internal.tests.qvt.oml.ui.AllTests\ Tests for org.eclipse.m2m.tests.qvt.oml.ui\ Operational QVT code completion\ general\ scr25027_1(org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest)\ scr25027_2(org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest)
It seems that the synchronize only solves concurrency, the race remains and may be resolved the wrong way.
By Ed Willink on Oct 11, 2017 06:58
(In reply to Ed Willink from comment #2)
With a bit of luck synchronizing QvtEnvironmentBase.doFindCollidingOperation may make the problem go away.
Except that I see intermittent failures on:
The synchronize seems better. May be it will do for JUnit testing.
The QvtReconciler concurrency issue is Bug 525857.
By Ed Willink on Feb 14, 2019 08:42
(In reply to Ed Willink from comment #2)
With a bit of luck synchronizing QvtEnvironmentBase.doFindCollidingOperation may make the problem go away.
No. This just reoccurred in a Gerrit build, even though doFindCollidingOperation is now synchronized.
By Ed Willink on Mar 17, 2019 04:06
Just happened again on a 3.9.2RC2 auto-rebuild.
Looking again, the comment#1 analysis looks plausible. There is a blackbox init concurrency.
By Ed Willink on Jun 01, 2021 06:33
Happened again while building 3.10.4RC1
blackboxCtxOper(org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest) Time elapsed: 2.483 s <<< ERROR!\ org.eclipse.emf.common.util.BasicEList$BasicIndexOutOfBoundsException: index=23, size=19\ at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest.initializeProposalProvider(CompletionTest.java:149)\ at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest.setUp(CompletionTest.java:101)
By Ed Willink on Sep 02, 2021 08:35
A slight variation occurred today.
org.eclipse.emf.common.util.BasicEList$BasicIndexOutOfBoundsException: index=23, size=19
at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest.initializeProposalProvider(CompletionTest.java:149)
at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest.setUp(CompletionTest.java:101)
This variation seems to be half way between this bug and Bug 526441, so they may actually be duplicates,
By Ed Willink on Oct 22, 2021 11:11
Bug 574706#c30 provides a plausible analysis of the overall symchronization inadequacies.
By Ed Willink on Apr 17, 2022 05:20
Just occurred again
org.eclipse.emf.common.util.BasicEList$BasicIndexOutOfBoundsException: index=41, size=37\ at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest.initializeProposalProvider(CompletionTest.java:149)\ at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest.setUp(CompletionTest.java:101)
Aug 22, 2022 11:25
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/195262
By Christopher Gerking on Aug 22, 2022 11:47
(In reply to Ed Willink from comment #4)
This just reoccurred in a Gerrit build, even though doFindCollidingOperation is now synchronized.
Didn't help because different threads use different versions of QvtEnvironmentBase.
The problem is that the QVTo reconciler thread and the main thread (which runs the UI tests) access the OCL standard library at the same time. In doing so, they both try to initialize the oclOperations() of a CollectionTypeImpl. Of course the initializations are not synchronized, which causes the test failure.
(In reply to Eclipse Genie from comment #10)
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/195262
Before running the completion tests, the proposed change triggers a "deep" initialization of the OCL/QVTo standard library, accessing the oclOperations() of all predefined types. This prevents the reconciler from restarting the initialization during the tests. The change also includes some further safeguards (disabling the auto build before every single test + waiting for the builder jobs to finish after disabling). But the major fix is the early initialization of the standard library.
Aug 24, 2022 07:36
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/195325
By Christopher Gerking on Aug 24, 2022 07:44
(In reply to Eclipse Genie from comment #12)
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/195325
To prevent similar problems, initialize additional stdlib classifiers by invoking methods such as oclIterators(), oclProperties() and so on. Perhaps this is not even necessary, just to make sure.
Aug 24, 2022 07:48
New Gerrit change created: https://git.eclipse.org/r/c/mmt/org.eclipse.qvto/+/195326
By Christopher Gerking on Jun 01, 2023 03:52
(In reply to Christopher Gerking from comment #11)
Before running the completion tests, the proposed change triggers a "deep" initialization of the OCL/QVTo standard library, accessing the oclOperations() of all predefined types. This prevents the reconciler from restarting the initialization during the tests. The change also includes some further safeguards (disabling the auto build before every single test + waiting for the builder jobs to finish after disabling). But the major fix is the early initialization of the standard library.
Any change to merge this to master?
Since only the tests are affected, this change does probably not justify a new release. Anyhow it would be good to merge because I think it would ensure reliability of our builds.
By Ed Willink on Nov 11, 2023 07:15
(In reply to Christopher Gerking from comment #15)
Any change to merge this to master?
Sorry. I missed this. The useless to me Eclipse Genie Gerrit messages just serve to occlude useful messages.
Great to have a fix for my 'favourite' bug.
Quick review shows it only affects the problematic test class so cannot do much harm.
Calls such as tupleType.oclProperties() certainly look like eager resolution; good.
But dictionaryType.getKeyType() looks like an EcoreUtil.resolveAll would be better.
Are the Job.getJobManager().join calls a debugging relic. If useful a comment would be good.
This is clearly a problem with the OCL Standard Library, so the CompletionTests fixup of the lazy library init should be promoted to a regular OCL Standard Library utility method. The laziness is clearly not threadsafe and although Eclipse OCL makes no claims to thread safety, it can try to be helpful. Is there a control path whereby the lazy init should have automatically given way to eager?
By Ed Willink on Nov 11, 2023 07:20
(In reply to Ed Willink from comment #16)
should be promoted to a regular OCL Standard Library utility method
OCL Bug 582625 raised.
To avoid a needless tight version coupling keep the CompletionTests solution, but mark it deprecated with a reference to the potential Bug 582625 utility solution.
By Ed Willink on Nov 11, 2023 09:31
Many thanks. Hopefully the builds will now be much more reliable.
Pushed to master for M3.
By Christopher Gerking on Nov 14, 2023 15:22
(In reply to Ed Willink from comment #18)
Hopefully the builds will now be much more reliable.
Unfortunately just happened to me again on a local Maven build (master branch, which includes the perceived fix).
By Ed Willink on Nov 15, 2023 03:24
Your fix was probably good but presumably not good enough.
I see two ways forward.
a) Study all EMF, OCL and QVTo code to identify all state that should be initialized before a concurrent thread is let loose. Then provide a good initialize helper.
b) Modify the tests to prevent use of concurrent threads.
Since none of EMF, OCL or QVTo projects claim thread safety, a) is an enhancement that may get a WONTFIX from any/all three projects.
So it has to b), perhaps with an assert to detect the concurrent thread activity e.g. a static variable identifying the thread permitted access to a QVToEnvironmentFactory / QVToAnalyzer / ...
By Christopher Gerking on Aug 07, 2024 06:36
(In reply to Ed Willink from comment #16)
But dictionaryType.getKeyType() looks like an EcoreUtil.resolveAll would be better.
Would EcoreUtil.resolveAll(stdLibModule.eResource()) be helpful to prevent the problem in a more general way?
By Ed Willink on Aug 07, 2024 07:01
(In reply to Christopher Gerking from comment #21)
(In reply to Ed Willink from comment #16)
But dictionaryType.getKeyType() looks like an EcoreUtil.resolveAll would be better. Would EcoreUtil.resolveAll(stdLibModule.eResource()) be helpful to prevent the problem in a more general way?
Proxies are a necessary evil during loading/unloading and lazy resolution can be efficient when few proxies need resolution or there are obscure dependencies. However for general full model use, eager resolution can avoid human debugging confusion and some issues whereby the internal varieties of eGet() need to pass an accurate resolve argument for iterations.
A QVTo transformation is generally a full model activity, so an eager resolveAll followed by a prompt diagnosis of any Resource.errors can help the user by giving prompt garbage-in diagnosis instead of waiting to report an obscure downstream problem.
By Ed Willink on Aug 07, 2024 07:10
(In reply to Ed Willink from comment #22)
diagnosis of any Resource.errors
NB This has to be transitive since, if A references B references C and a proxy to C fails, the Resource.errors for B rather than A contains the diagnostic.
All ResourceSet Resource errors is often easier than precise transitive.
By Christopher Gerking on Aug 21, 2024 07:26
I guess my current solution was only half-baked. The original problem was caused by EOperations such as oclOperations(), oclIterators() and oclProperties(). They need to be invoked early, as otherwise the initialization inside of these methods is executed multiple times on different threads.
For the sake of completeness, we should perhaps invoke predefinedType.getName() as well because getName() is an explicit EOperation of PredefinedType
But similar problems might be actually caused not by EOperations but also with the initialization of EStructuralFeatures. That is why some of them have been explicitly accessed/initialized, such as collectionType.getElementType(), but others not. The proper initialization of all EStructuralFeatures should be accomplished consistenly by invoking EcoreUtil.resolveAll(...) on the stdlib module. Pushed to cgerking/525852.
By Ed Willink on Aug 21, 2024 07:51
(In reply to Christopher Gerking from comment #24)
I guess my current solution was only half-baked. ... proper initialization
OCL initialization is difficult because there is no concept of an EMF-Application whose start could have a hook to start/stop OCL in a timely fashion. Instead OCL may need to start lazily whem some OCL delegate is executed within an apprently simple EMF model. Worse OCL may need GC/finalize() to stop.
QVTo initialization should be easy because there is a limited number of startUp/tearDown paths such as TransformationExecutor.
I suggest developing a family of assertXXX routines to check some theory such as the-name-is-already-not-null and scatter the code with
assert assertXXX(...)
and just run the test suites to see whether some theory is contradicted.
assertXXX returns true or throws AssertionFailedError, so that it costs non -ea users nothing if the assertions are accidentally left in.
It should be possible to localize all initialization actions in a startup module/routine and then eliminate the redundant re-initializations elsewhere.
Maybe the final line of stsrtUp code is:
assert assertInitializationIsComplete(executor);
By Ed Willink on Aug 22, 2024 06:13
(In reply to Christopher Gerking from comment #24)
EcoreUtil.resolveAll(...) on the stdlib module. Pushed to cgerking/525852.
This looks like a good idea,. but...
(The promotion of an init to OCL as part of 582625 remains a good suggestion)
The init should for now be a QVTo utility, since, if something is needed to init test correctly, why is it not available/used by regular code?
(For Xtext-based OCL/QVTd the Xtext StandaloneSetup policy is extended to provide e.g.org.eclipse.qvtd.xtext.qvtrelation.QVTrelationStandaloneSetup.init() called for test purposes from org.eclipse.qvtd.xtext.qvtbase.tests.utilities.XtextCompilerUtil.doQVTrelationSetup().)
By Ed Willink on Aug 26, 2024 03:01
It's RC1 day for QVTo tomorrow.
The extra "Resolve all contents of standard library" affects only test code so do you want it 3.11.0RC1 or do you want to experiment further?
By Christopher Gerking on Aug 26, 2024 05:54
(In reply to Ed Willink from comment #26)
(The promotion of an init to OCL as part of 582625 remains a good suggestion)
Definitely.
The init should for now be a QVTo utility, since, if something is needed to init test correctly, why is it not available/used by regular code?
Good point. However, in the cases I stumbled across, the cause was actually OCL's eOperations mentioned above, which trigger initializations of variables. For now, invoking the EcoreUtil.resolveAll(...) on the QVTo stdlib was only a precaution to verify whether it actually helps prevent the problem.
QVTo has no dedicated standalone setup, so if actually helpful, we would need to invoke such a utility inside the TransformationExecutor class.
(In reply to Ed Willink from comment #27)
The extra "Resolve all contents of standard library" affects only test code so do you want it 3.11.0RC1 or do you want to experiment further?
Well, let the CI do the further experiments. That's why I would like to include the change in RC1.
I was trying to make it more generic by invoking all zero-param eOperations of the stdlib and all its children:
for (Iterator
But when doing so, listTypeImpl.eInvoke(oclOperations, ECollections.emptyEList()) results in an IndexOutOfBoundsException because eInvoke(oclOperations, arguments) mistakenly delegates to isInstance(arguments.get(0)). Is it a bug or am I doing something wrong?
By Ed Willink on Aug 26, 2024 06:28
(In reply to Christopher Gerking from comment #28)
QVTo has no dedicated standalone setup, so if actually helpful, we would need to invoke such a utility inside the TransformationExecutor class.
Almost every non-trivial project has multiple initializations that the user must provide. These can be painful for the user to discover. Worse, a future QVTo release may require more/changed initialization requiring action by all users.
cf the very belated UMLResourcesUtil.init() introduced to avoid the pain of UML 2.5 initialization (and one day completely hide UIML 2.6 etc).
(In reply to Ed Willink from comment #27) Well, let the CI do the further experiments. That's why I would like to include the change in RC1.
I'm not keen on committing an experiment which as indicated below is still a wip. \
ECollections.emptyEList()) results in an IndexOutOfBoundsException because eInvoke(oclOperations, arguments) mistakenly delegates to isInstance(arguments.get(0)). Is it a bug or am I doing something wrong?
Yes, you are definitely doing something wrong. It is well used.
eInvoke like newInstance has a varargs and so can be an enumerated list or an array of arguments. It cannot be a Collection.
I was trying to make it more generic by invoking all zero-param eOperations of the stdlib and all its children:
Generic is good but I'm not convinced that "zero-param eOperations" is a sufficiently accurate predicate.
Classic OCL is very very stable so I suspect a hard-coded list of methods would be safer. (For test purposes you could 'printf' an autogenerated list of candidates for comparison.)
By Christopher Gerking on Aug 26, 2024 07:02
(In reply to Ed Willink from comment #29)
I'm not keen on committing an experiment which as indicated below is still a wip.
I see.\
eInvoke like newInstance has a varargs
No, it's eInvoke(EOperation operation, EList<?> arguments).
Classic OCL is very very stable
Yes but the problem might be in ImperativeOCL? For me the mismatch occurs when invoking getName() (with operation ID 0) on a ListTypeImpl. It inherits from CollectionTypeImpl which does not override eInvoke(...), so the actual delegation only involves methods of EClassifierImpl. Here, operation ID 0 refers to getEAnnotation() rather than getName(). Why do they have the same ID?
By Ed Willink on Aug 26, 2024 07:22
(In reply to Christopher Gerking from comment #30)
eInvoke like newInstance has a varargs No, it's eInvoke(EOperation operation, EList<?> arguments).
Oops. Yes. Searching my code:
org.eclipse.ocl.pivot.internal.library.EInvokeOperation
has two calls both of which have a list with two null first arguments. You probably have a misalgned list.
Classic OCL is very very stable Yes but the problem might be in ImperativeOCL? For me the mismatch occurs when invoking getName() (with operation ID 0) on a ListTypeImpl. It inherits from CollectionTypeImpl which does not override eInvoke(...), so the actual delegation only involves methods of EClassifierImpl. Here, operation ID 0 refers to getEAnnotation() rather than getName(). Why do they have the same ID?
I don't understand the problem. Can you point me to a JUnit test that demonstrates what is not working?
By Christopher Gerking on Aug 26, 2024 08:15
(In reply to Ed Willink from comment #31)
I don't understand the problem. Can you point me to a JUnit test that demonstrates what is not working?
Yes, run org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTests on cgerking/525852. It leads to an initialization error caused by an IndexOutOfBoundsException, which is obviously due to mismatching operation IDs. The invocation of getName() delegates to getEAnnotation(String) as both are assigned an ID of 0.
By Ed Willink on Aug 26, 2024 08:57
Great. A repro makes things so much easier.
Show the transitive type hierarchy of ListTypeImpl. eInvoke is declared for EClassifierIMpl, EModelElementImpl etc, but oops not for CollectionTyeImpl (OCL limitation) or ListTypeImpl (QVTo limitation).
I suspect that the design decision was taken that the extra overhead of eInvoke generation from genmodel was not worth the effort; nobody wanted it. (Fix is to set Model->Operation Reflection in the *.genmodel and regenerate.)
EMF often chooses not to inflict overheads on all users solely to diagnose 'stupid users' so there is no EMF diagnosis of your incompatible eInvoke() call.
--
This wasn't actually the problem I didn't understand.
Why are there unresolved proxies that your resolveAll call helps with?
By Christopher Gerking on Aug 26, 2024 09:36
(In reply to Ed Willink from comment #33)
This wasn't actually the problem I didn't understand.
Why are there unresolved proxies that your resolveAll call helps with?
Perhaps there are none. This was basically an attempt to replace the explicit resolution of eStructuralFeatures like collectionType.getElementType(), which I introduced in commit bf747cc5eb7077d3084dcd60547581c4846412f9 two years ago.
But perhaps this was always totally useless. The actual problem did not occur with eStructuralFeatures but with eOperations like oclIterators() which must be invoked explicitly. Even the resolveAll call does not take this responsibility.
By Ed Willink on Aug 26, 2024 09:41
(In reply to Ed Willink from comment #33)
I suspect that the design decision was taken that the extra overhead of eInvoke generation from genmodel was not worth the effort; nobody wanted it.
Bug 255469#c23 is the definitive decision. It seems to have rippled unpleasantly.
Subsequently Bug 255469 reports an important fix.
Maybe Classic OCL and QVTo should revisit. UML2 and Pivot OCL have eInvoke.
The API change wrt EModelElement not inheriting from EObject model-wise is already in so further incompatibilities may be within the bounds of what we can accommodate with an API filter.
By Ed Willink on Aug 26, 2024 09:42
(In reply to Christopher Gerking from comment #34)
Perhaps there are none. This was basically an attempt to replace the explicit resolution of eStructuralFeatures like collectionType.getElementType(), which I introduced in commit bf747cc5eb7077d3084dcd60547581c4846412f9 two years ago.
OK. Is there a repro demonstrating the need for collectionType.getElementType()?
By Christopher Gerking on Aug 26, 2024 09:51
(In reply to Ed Willink from comment #36)
OK. Is there a repro demonstrating the need for collectionType.getElementType()?
No. The original repro was with predefinedType.oclOperations().
By Ed Willink on Aug 26, 2024 10:01
eInvoke() is perhaps a red herring. eInvoke() provides support for invoking EOperations. It does not provide support for accessing EStructuralFeatures. You access collectionType.getElementType() via eGet() not eInvoke().
?? collectionType.getElementType() might be an unresolved proxy if the OCL standard library was (inefficiently) loaded from /org.eclipse.ocl.ecore/model/oclstdlib.ecore rather than the built-in code.
The first ten lines of init() in /org.eclipse.ocl.ecore/src/org/eclipse/ocl/ecore/internal/OCLStandardLibraryImpl.java make the static/dynamic load decision. I doubt QVTo supports a dyanmic QVTo standard library.
By Christopher Gerking on Aug 26, 2024 10:11
(In reply to Christopher Gerking from comment #37)
The original repro was with predefinedType.oclOperations().
But even with this "fix" included, it just happended again:
bugzilla2639_1(org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest) Time elapsed: 0.139 s <<< ERROR!\ org.eclipse.emf.common.util.AbstractEList$BasicIndexOutOfBoundsException: index=2, size=2\ at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest.initializeProposalProvider(CompletionTest.java:160)\ at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest.setUp(CompletionTest.java:112)
By Ed Willink on Aug 26, 2024 10:19
Contrasting QvtOperationalStdLibrary with OCLStandardLibraryImpl seems to reveal another example of the problem that the QVTo developers were extending OCL but failing to negotiate any difficulties they were having. Consequently there are many really kludgy workarounds.
It looks to me as if QvtOperationalStdLibrary should extend OCLStandardLibraryImpl, which as a minimum requires that the OCLStandardLibraryImpl constructor is protected rather than private. A few other protected methods could make the extension clean.
Sometimes it is a good idea to abandon the kludge fight.
By Ed Willink on Aug 26, 2024 10:59
(In reply to Christopher Gerking from comment #39)
See https://ci.eclipse.org/qvt-oml/job/qvto-branch-tests/337/testReport/junit/ org.eclipse.m2m.internal.tests.qvt.oml.ui.completion/CompletionTest/ bugzilla2639_1/
I cannot reproduce this in my workspace, but I can get a solid three fails. On further investigation there is a problem that requires each of the org.eclipse.m2m.qvt.oml.examples.blackbox, org.eclipse.m2m.qvt.oml.samples.simpleuml2rdb projects to be closed. It would seems that the diagnosis in the CompletionTest constructor needs more work.
By Ed Willink on Aug 26, 2024 11:28
(In reply to Christopher Gerking from comment #39)
org.eclipse.emf.common.util.AbstractEList$BasicIndexOutOfBoundsException: index=2, size=2 at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest. initializeProposalProvider(CompletionTest.java:160) at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest. setUp(CompletionTest.java:112)
is impossible.
CompletionTest.java:112 looks right but
CompletionTest.java:160 is
ICompletionProposal[] proposals = processor.computeCompletionProposals((ITextViewer) sourceViewer, myOffset);
which involves no EList so cannot throw AbstractEList$BasicIndexOutOfBoundsException.
Indeed there is little sign of any ELists in the nearby code or computeCompletionProposals.
How does the impossible happen?
The full console log is no more informative so I suspect that the stack trace has been filletted; a BasicIndexOutOfBoundsException deep within computeCompletionProposals somehow escaped through finally's without filling in the trace. TODO study the computeCompletionProposals call tree to find ELists.
By Christopher Gerking on Aug 26, 2024 12:46
(In reply to Ed Willink from comment #42)
How does the impossible happen?
See comment #0 for a strack trace from CompletionTest.initializeProposalProvider via CollectionTypeImpl.oclOperations to BasicEList.remove... In this case, the problem was triggered by loading a Java blackbox (which is hopefully not the main cause, but of course cannot be ruled out either).
Adding a breakpoint to CollectionTypeImpl.oclOperations() shows that there are more instances of CollectionTypeImpl beyond the OCL stdlib. In particular, there are instances of SequenceTypeImpl or SetTypeImpl contained as eClassifiers inside an OperationalTransformationImpl. Why is that? Of course our current solution is restricted to the stdlib types.
By Ed Willink on Aug 26, 2024 12:59
I suspect that your fixes have moved the problem around, but that it still has some concurrent aspect at its heart.
I think black boxes may be a red herring. The observations on the reconciler seem more apposite. If the reconciler is improving things it could well be messing them up concurrently. Why is it running? Ensure that it is synchronized. Run a libraries-are-consistent check before and after reconcilation.
I tried instrumenting the basic OCLStandardLibraryUtil.createXXXIterations/Operations to printf thread role and id. Everything seemed very well behaved. However I note that the static createXXX operations return a new List each time rather than a stable static list. As soon as I try to understand the QVTo code I hit another depressing kludge; everything is wrapped in duplicates. I find the removeAll at the heart of the original report very disconcerting.
I suspect the only way forward is to saturate the code with printf's so that every operation creation/addition/removal can be associated with a time and thread. Nothing should happen after the first test since everything should be constructed once before start.
By Ed Willink on Aug 26, 2024 13:09
(In reply to Ed Willink from comment #44)\
The observations on the reconciler seem more apposite.
I think this too is a red herring.
Instrumenting QvtReconciler, a new instance is created installed and uninstalled for each completion test, but always on the main thread. It is a source text facility well removed from the models. doForceReconciling never happens.
By Christopher Gerking on Aug 27, 2024 06:07
Created attachment 289476 (attachment deleted)\
CompletionTests Stack
By Christopher Gerking on Aug 27, 2024 06:29
(In reply to Ed Willink from comment #44)
I suspect that your fixes have moved the problem around, but that it still has some concurrent aspect at its heart.
Reproduced. The main thread and the QvtReconciler daemon thread can still access the oclOperations() of one and the same CollectionTypeImpl concurrently (see stack in attachment 289476). I hd hoped that it would be possible to disable the reconciler (by joining the builder jobs). But apparently these are two different things, at least it didn't solve the problem.
QvtEnvironmentBase.doFindCollidingOperation(...) is already synchronized, but this does not solve the problem either because it is invoked on two separate instances of QvtEnvironmentBase (passing the same CollectionTypeImpl as ownerType).
Accordingly, the ownerType param must be used for synchronization instead. This could be achieved by wrapping up the invocation of TypeUtil.getOperations(this, ownerType) in a synchronized(ownerType) statement.
By Ed Willink on Aug 27, 2024 06:36
Well done. A clear demonstration of concurrency that neither EMF nor OCL supports.
To get away with it we need to ensure that doFindCollidingOperation is immutable, i.e. it returns a reference to an already fully defined operation and has no side effects.
org.eclipse.ocl.pivot.internal.resource.ASResourceImpl.setSaveable has code to install an ImmutabilityCheckingAdapter which I wrote because ResourceImpl.ModificationTrackingAdapter didn't suit my purposes.
A derived ModificationTrackingAdapter that is installed everywhere but can be switched on and off around doFindCollidingOperation might uncover the mutation.
By Christopher Gerking on Aug 27, 2024 06:54
Created attachment 289480 CompletionTests Stack
:notepad_spiral: CompletionTests.txt
By Christopher Gerking on Aug 27, 2024 06:56
Comment on attachment 289476 (attachment deleted)\
CompletionTests Stack
Replaced by attachment 289480 (attachment deleted) which includes the full stack up to CollectionTypeImpl.oclOperations()
| --- | --- | | Bugzilla Link | 525852 | | Status | REOPENED | | Importance | P3 normal | | Reported | Oct 11, 2017 05:47 EDT | | Modified | Aug 28, 2024 09:50 EDT | | Depends on | 583533, 582625 | | See also | 525857, 526641, 574706, Gerrit change 195262, Gerrit change 195325, Gerrit change 195326, 255469 | | Reporter | Ed Willink |
Description
When running the UI tests using the Tycho Surefire plugin the first UI test is intermittent. Failure trace below. Despite the first test failing subsequent tests succeed.
Intermittency suggests a SEt ordering dependency.
From the stack trace the removal of entries from CollectionTypeImpl.oclOperations looks very very smelly.
-------------------------------------------------------------------------------\ Test set: org.eclipse.m2m.internal.tests.qvt.oml.ui.AllTests\ -------------------------------------------------------------------------------\ Tests run: 202, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 32.152 sec <<< FAILURE! - in org.eclipse.m2m.internal.tests.qvt.oml.ui.AllTests\ blackboxCtxOper(org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest) Time elapsed: 1.125 sec <<< ERROR!\ org.eclipse.emf.common.util.BasicEList$BasicIndexOutOfBoundsException: index=47, size=37\ at org.eclipse.emf.common.util.BasicEList.remove(BasicEList.java:601)\ at org.eclipse.emf.common.util.AbstractEList.removeAll(AbstractEList.java:486)\ at org.eclipse.ocl.ecore.impl.CollectionTypeImpl.oclOperations(CollectionTypeImpl.java:449)\ at org.eclipse.ocl.ecore.impl.CollectionTypeImpl.oclOperations(CollectionTypeImpl.java:452)\ at org.eclipse.ocl.AbstractTypeChecker.getOperations(AbstractTypeChecker.java:710)\ at org.eclipse.ocl.util.TypeUtil.getOperations(TypeUtil.java:132)\ at org.eclipse.m2m.internal.qvt.oml.ast.env.QvtEnvironmentBase.doFindCollidingOperation(QvtEnvironmentBase.java:611)\ at org.eclipse.m2m.internal.qvt.oml.ast.env.QvtEnvironmentBase.findCollidingOperation(QvtEnvironmentBase.java:565)\ at org.eclipse.m2m.internal.qvt.oml.ast.env.QvtOperationalEnv.defineImperativeOperation(QvtOperationalEnv.java:689)\ at org.eclipse.m2m.internal.qvt.oml.blackbox.java.OperationBuilder.createOperation(OperationBuilder.java:165)\ at org.eclipse.m2m.internal.qvt.oml.blackbox.java.OperationBuilder.buildOperation(OperationBuilder.java:49)\ at org.eclipse.m2m.internal.qvt.oml.blackbox.java.JavaModuleLoader.loadModule(JavaModuleLoader.java:101)\ at org.eclipse.m2m.internal.qvt.oml.blackbox.java.JavaBlackboxProvider$JavaUnitDescriptor.load(JavaBlackboxProvider.java:197)\ at org.eclipse.m2m.internal.qvt.oml.compiler.BlackboxUnitResolver$BlackboxUnitContents.loadElements(BlackboxUnitResolver.java:160)\ at org.eclipse.m2m.internal.qvt.oml.compiler.BlackboxUnitResolver$BlackboxUnitProxy.load(BlackboxUnitResolver.java:115)\ at org.eclipse.m2m.internal.qvt.oml.compiler.QVTOCompiler.doCompile(QVTOCompiler.java:353)\ at org.eclipse.m2m.internal.qvt.oml.compiler.QVTOCompiler.doCompile(QVTOCompiler.java:414)\ at org.eclipse.m2m.internal.qvt.oml.compiler.QVTOCompiler.compileSingleFile(QVTOCompiler.java:323)\ at org.eclipse.m2m.internal.qvt.oml.compiler.QVTOCompiler.compile(QVTOCompiler.java:202)\ at org.eclipse.m2m.internal.qvt.oml.compiler.QVTOCompiler.compile(QVTOCompiler.java:217)\ at org.eclipse.m2m.internal.qvt.oml.editor.ui.completion.QvtCompletionCompiler.compileAll(QvtCompletionCompiler.java:109)\ at org.eclipse.m2m.internal.qvt.oml.editor.ui.completion.QvtCompletionData.getEnvironment(QvtCompletionData.java:147)\ at org.eclipse.m2m.internal.qvt.oml.editor.ui.completion.LightweightParserUtil.getOclExpression(LightweightParserUtil.java:237)\ at org.eclipse.m2m.internal.qvt.oml.editor.ui.completion.LightweightParserUtil.getOclExpression(LightweightParserUtil.java:224)\ at org.eclipse.m2m.internal.qvt.oml.editor.ui.completion.collectors.AbstractCallExpSourceCollector.getCallExpSource(AbstractCallExpSourceCollector.java:72)\ at org.eclipse.m2m.internal.qvt.oml.editor.ui.completion.collectors.AccessorCollector.getCallExpSource(AccessorCollector.java:42)\ at org.eclipse.m2m.internal.qvt.oml.editor.ui.completion.collectors.AbstractCallExpSourceCollector.isApplicableInternal(AbstractCallExpSourceCollector.java:43)\ at org.eclipse.m2m.internal.qvt.oml.editor.ui.completion.collectors.AbstractCollector.isApplicable(AbstractCollector.java:25)\ at org.eclipse.m2m.internal.qvt.oml.editor.ui.completion.QvtCompletionProcessor.updateCategoryIndex(QvtCompletionProcessor.java:164)\ at org.eclipse.m2m.internal.qvt.oml.editor.ui.completion.QvtCompletionProcessor.computeCompletionProposals(QvtCompletionProcessor.java:122)\ at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest.initializeProposalProvider(CompletionTest.java:134)\ at org.eclipse.m2m.internal.tests.qvt.oml.ui.completion.CompletionTest.setUp(CompletionTest.java:86)