Closed emammoser closed 1 year ago
I've been able to reproduce this, although not consistently, with a small model (BasicPubSub).
The exception occurs because the UML file is dirty and then Papyrus tries to refresh its editors and views, in particular the ModelExplorer. This results in the exception thrown multiple times, and sometimes this results in the ModelExplorer view being empty, but not always. I have not been able to determine the precise conditions for the ModelExplorer view to be blank, though.
The exception, at least, seems to be a long-standing problem in Papyrus itself. Here are several references to it:
According to some of these discussions and to the Papyrus code itself, it seems like in some cases at least, the exception itself is "fine". But I'm not really sure the Papyrus developers have actually run into scenarios like ours.
We also dealt with this exception some time ago, in Issue #136 and then in Issue #150. The solution there registered a dummy service for the IMultiDiagramEditor. In theory this should get rid of the exception, but for some reason it doesn't.
Also, we don't know with absolute certainty that that exception directly causes the blank Model Explorer, but it is reasonable to think it is.
I'll continue to investigate.
If you find a predictable way to reproduce the blank Model Explorer, I'd be very interested in knowing about it.
I have added a new model to the bugs folder in cx-reproducers. The folder is 466 and contains two zip files.
SPDM_Large.zip This model is the SignalProcessingDataModel with many of the modules duplicated many times to replicate a large model. This model appears to have multiple issues including the one mentioned in this issue. You should be able to recreate the issue when modifying one of the port message data type definitions. Click the pencil and change the type of a random type and then save). In addition to the bug mentioned in this issue, we see a few other segfault errors as well as a problem that has been plaguing our team for years even back in RSA. This bug is one where the model saving goes wrong and some portion of the model is saved to UML twice giving invalid XML. I have added an example of this corrupted model in the other zip file.
SPDM_Large_corrupted.zip -This contains the corrupted UML. Notice that the models XML ends with a </xmi:XMI> tag, but then contains what appears to be a trailing portion of the original model after it. If you delete all contents after the first ending </xmi:XMI> tag, the model is valid again.
Thanks. I'll take a look at them.
I can reproduce it with the large model, but I need to have two models opened, make several edits and then switch back-and-forth between the two Papyrus editors and then do a "Save All". Is this the case for you as well? Multiple models opened?
There is something a bit strange. When I run into this exception, which happens when trying to save the model and it goes through the Papyrus views and editors, in particular the Model Explorer, the path that the code follows seems to suggest that Papyrus handles the exception and creates the so-called "SaveAndDirty" service, which it uses to save models listening to their "dirty" state, so that part works, but it is not clear why does it fail to render the contents of the Model Explorer after that. It does seem to be a bug in core Papyrus.
No, I can reproduce it with only one model. Most of the time it only takes one or two saves after changing a type definition for the the empty model explorer bug to occur. However, I do only test on virtualized Alma8 Linux machines.
Hmmm. I cannot reproduce it with only one model open, including on a CentOS VM. I wonder what you may have that's different from my environment or if it is related to specific elements or actions.
Nevertheless, I can reproduce it as I mentioned before, by modifying the model externally and switching between editors. Since the exception is the same and the side-effect (blank Model Explorer) is the same, I assume the cause is the same as what you experience. I'll continue trying to reproduce it as you did, but I'll focus on the behaviour that I can reproduce.
I'm adding some observations and hypothesis here, to document what's going on.
After the
org.eclipse.papyrus.infra.core.services.ServiceNotFoundException: No service registered under 'interface org.eclipse.papyrus.infra.ui.editor.IMultiDiagramEditor'
exception, the value of saveable
is null
where part
= org.eclipse.papyrus.views.modelexplorer.ModelExplorerPageBookView@xxxxxx
at
org.eclipse.ui.internal.WorkbenchPage.getDirtyParts()
Presumably this means that the ModelExplorer
is not included in the list of dirty parts and therefore not redrawn?
When exception
org.eclipse.papyrus.infra.core.services.ServiceNotFoundException: No service registered under 'interface org.eclipse.papyrus.infra.ui.editor.IMultiDiagramEditor'
is thrown at
org.eclipse.papyrus.infra.core.services.ServicesRegistry.getService(Class<S>)
the maps namedServices
and addedServices
of the ServiceRegistry
are empty.
Looks like the ServiceRegistry
's maps are cleared when
org.eclipse.papyrus.infra.ui.editor.CoreMultiDiagramEditor.doReload()
calls deactivate()
.
Perhaps the exception occurs because ServicesRegistry.getService
gets called before reactivating on reload. Maybe a possible race condition.
Why do we try to call ServicesRegistry.getService
before reactivating?
The following is an abbreviated sequence of events after modifying on a second window, clicking "Save All", and switching to the opened Papyrus editor:
PapyrusMultiDiagramEditor.doReload()
is invoked.deactivate()
which disposes of its ServicesRegistry
, in particular the service associated to IMultiDiagramEditor
, which is this instance of PapyrusMultiDiagramEditor
.initContents()
which invokes loadModelAndServices()
ServicesRegistry
associated to this instance of PapyrusMultiDiagramEditor
,
and registers this instance of PapyrusMultiDiagramEditor
as a service for IMultiDiagramEditor
activate()
event.dispatchEditorReloaded(listeners)
where event
is an instance of EditorReloadEvent
org.eclipse.ui.part.WorkbenchPart.firePropertyChange(int)
on the PapyrusMultiDiagramEditor
instance.org.eclipse.ui.internal.WorkbenchPage.getDirtyParts()
where part
is the ModelExplorerPageBookView
instanceorg.eclipse.papyrus.views.modelexplorer.ModelExplorerView.getAdapter(Class)
on the active ModelExplorerView
instance...IMultiDiagramEditor
service from the ServiceRegistry
(serviceRegistry.getService(IMultiDiagramEditor.class);
)but the ModelExplorerView
instance still has a reference to the old ServiceRegistry
which had been cleared during deactivation!!!
This definitely looks like a bug in the Papyrus core.
Thank you for your investigation. Do you know of a way that you could fix it in papyurs? Unfortunately, this bug happens pretty frequently with our production models.
Not yet. That's what I'm investigating today.
There may be a relatively simple solution, but it does require a patch to Papyrus. I'm trying to implement it now.
More details on why this occurs:
In the previous sequence, during PapyrusMultiDiagramEditor.doReload()
and before deactivate()
it calls event.dispatchEditorAboutToReload(listeners);
which invokes editorAboutToReload()
on the reloadAdapter
of the ModelExplorerView
. This invokes ModelExplorerView.deactivate()
, which calls reloadableEditor.removeEditorReloadListener(reloadAdapter);
and so it would appear that after the reload, the method editorReloaded(...)
of the reloadAdapter
is not executed, and therefore, the ModelExplorerView
doesn't update its serviceRegistry
to the new one.
Or is this what happens? Well, no. Both calls to event.dispatchEditorAboutToReload(listeners);
and event.dispatchEditorReloaded(listeners);
have the same list of listeners that includes the reloadAdapter
of the ModelExplorerView
. But the reloadAdapter
of the PapyrusMultiDiagramEditor
is invoked first, which leads to reload resources which leds to a call to WorkbenchPart.getDirtyParts()
which leads to ModelExplorerView.getAdapter
where it will still have the old serviceRegistry
that has been emptied and disposed.
So the problem is that the editorReloaded()
method of the reloadAdapter
of the PapyrusMultiDiagramEditor
is invoked before
the editorReloaded()
method of the reloadAdapter
of the ModelExplorerView
, and this is because the later appears after the former in the list of listeners
in PapyrusMultiDiagramEditor.doReload()
.
This list of listeners
is an immutable copy of the reloadListeners
of the PapyrusMultiDiagramEditor
, and the listener for the ModelExplorerView
is added during its creation/initialization, which happens after the PapyrusMultiDiagramEditor
is created.
Well, my first attempted solution does not work. I'm able to suppress the exception, by fixing the service registry reference in the ModelExplorerView, but unfortunately it still fails to display its contents and is blanked out. This means that my hypothesis that the exception lead to the blank view was incorrect. I'll have to continue digging.
Thank you for the update
Another important update: the problem occurs in plain Papyrus without CX installed at all.
So we can rule out anything in CX as causing the problem.
I also tried it on the latest version of Papyrus (6.5.0 released on June 14th, 2023) and I was not able to reproduce the problem, so it may have been fixed in more recent versions of Papyrus.
This suggests that a path forward may be upgrading to the latest Papyrus. Of course, that's not necessarily easy, as many of our plugins have very old dependencies so it wouldn't be guaranteed to work. Furthermore, I imagine that your own plugins may have old dependencies too. Nevertheless, is upgrading Papyrus something that you would consider?
And I just found a bugzilla entry for a Papyrus bug that sounds exactly like what we are seeing: Bug 574410. It was reported and fixed in 2021 so it was after 4.8 which is the version that we are based on. I'll see if I can back-port the solution given there.
This suggests that a path forward may be upgrading to the latest Papyrus. Of course, that's not necessarily easy, as many of our plugins have very old dependencies so it wouldn't be guaranteed to work. Furthermore, I imagine that your own plugins may have old dependencies too. Nevertheless, is upgrading Papyrus something that you would consider?
@j26151 can correct me if I'm wrong but I think the only reason we're locked to this Papyrus is because you (CX) deliver your code with it. We just depend on Java 11 (minimum). I don't believe we have any hard dependencies on older packages.
For now though, to get a quick fix in on our current baseline, I would say continue with the backport. I'd love to have some future time spent looking at that Papyrus upgrade though. The more we can stay current with the latest the better off we'll be I'd imagine as they'll be fixing community found bugs like this.
@J17359 Is right, we would have no problems moving to a newer version of papyrus on the head of the papyrus branch, however, since we are seeing this issue on the previous maintenance streams (which are what our users are user now and for the foreseeable future), we really need a patch of some sort to provide them.
Ok, the back-port seems to work. You might want to test the build artifacts once the build is done. The link is here.
As per your comments, I assume you want this in both the 2.4 and 2.3 maintenance branches as well, correct?
Yes please. I will test the back-port tomorrow morning. Thank you.
I tested this out this morning and I was not able to recreate the initial issue of the model explorer being empty after saving. Great work! Let's definitely get this patched back to 2.4 and 2.3. Thank you.
As a side note, were you ever able to recreate the model corruption issue that I mentioned with the large SPDM example? I did see that issue still occurs with very large models. Since most of our developers are not seeing it (yet), we don't need to prioritize solving that issue now, but I am curious if you ever saw the corruption yourself?
The fix for this issue has been reviewed by Paul and merged into the papyrus branch as well as the 2.3 and 2.4 maintenance branches.
I've opened Issue #470 to address with the corruption that you are seeing. Let's continue the discussion over there.
As for this one, let me know if we can close it now.
I agree, we can close this issue.
Issue and tracking information
Developer's time Estimated effort to fix (hours):
Developer's Actual time spent on fix (hours)
Issue reporter to provide a detailed description of the issue in the space below
When making a change and saving a model of significant size, there is a 25% chance (or greater) that the model explorer becomes empty. The only solution to which is to close and re-open the model. If I am making numerous changes to the model this can become quite cumbersome constantly requiring us to close and reopen the model.
Unfortunately, it appears that it requires models of a certain size and complexity and non of our examples are currently large enough to cause this issue. Thus, we do not have a reproducer for this yet. However, we do notice that we get a specific stack trace that is hidden in the Error view. It appears that your team has multiple posts/questions about this error already out on the internet but I didn't really see any resolutions.
The exception is:
eclipse.buildId=unknown java.version=11.0.19 java.vendor=Red Hat, Inc. BootLoader constants: OS=linux, ARCH=x86_64, WS=gtk, NL=en_US Framework arguments: -product com.zeligsoft.papyrus-cx.axcioma.rcp.product Command-line arguments: -product com.zeligsoft.papyrus-cx.axcioma.rcp.product -data /tmp/papyrus_bug -dev file:/tmp/papyrus_bug_sdk/.metadata/.plugins/org.eclipse.pde.core/New_configuration/dev.properties -os linux -ws gtk -arch x86_64 -consoleLog
org.eclipse.papyrus.views.modelexplorer Error Fri Jul 21 09:51:00 EDT 2023 Unexpected Error
org.eclipse.papyrus.infra.core.services.ServiceNotFoundException: No service registered under 'interface org.eclipse.papyrus.infra.ui.editor.IMultiDiagramEditor' at org.eclipse.papyrus.infra.core.services.ServicesRegistry.getService(ServicesRegistry.java:410) at org.eclipse.papyrus.views.modelexplorer.ModelExplorerView.getAdapter(ModelExplorerView.java:1127) at org.eclipse.papyrus.views.modelexplorer.core.ui.pagebookview.ViewPartPage.getAdapter(ViewPartPage.java:140) at org.eclipse.core.runtime.Adapters.adapt(Adapters.java:63) at org.eclipse.core.runtime.Adapters.adapt(Adapters.java:112) at org.eclipse.ui.part.PageBookView.getAdapter(PageBookView.java:532) at org.eclipse.papyrus.views.modelexplorer.ModelExplorerPageBookView.getAdapter(ModelExplorerPageBookView.java:130) at org.eclipse.core.runtime.Adapters.adapt(Adapters.java:63) at org.eclipse.core.runtime.Adapters.adapt(Adapters.java:112) at org.eclipse.ui.internal.SaveableHelper.getSaveable(SaveableHelper.java:467) at org.eclipse.ui.internal.WorkbenchPage.getDirtyParts(WorkbenchPage.java:3532) at org.eclipse.ui.internal.handlers.SaveAllHandler.evaluate(SaveAllHandler.java:75) at org.eclipse.ui.internal.handlers.AbstractSaveHandler$1.evaluate(AbstractSaveHandler.java:55) at org.eclipse.ui.internal.services.EvaluationReference.evaluate(EvaluationReference.java:74) at org.eclipse.ui.internal.services.EvaluationReference.evaluate(EvaluationReference.java:100) at org.eclipse.ui.internal.services.EvaluationService.requestEvaluation(EvaluationService.java:290) at org.eclipse.ui.internal.handlers.DirtyStateTracker.update(DirtyStateTracker.java:49) at org.eclipse.ui.internal.handlers.DirtyStateTracker.propertyChanged(DirtyStateTracker.java:111) at org.eclipse.ui.part.WorkbenchPart.firePropertyChange(WorkbenchPart.java:124) at org.eclipse.ui.views.properties.PropertySheet.updateContentDescription(PropertySheet.java:554) at org.eclipse.ui.views.properties.PropertySheet.showSelectionAndDescription(PropertySheet.java:566) at org.eclipse.ui.views.properties.PropertySheet.selectionChanged(PropertySheet.java:544) at org.eclipse.ui.internal.e4.compatibility.SelectionService.notifyListeners(SelectionService.java:266) at org.eclipse.ui.internal.e4.compatibility.SelectionService.notifyListeners(SelectionService.java:180) at org.eclipse.ui.internal.WorkbenchPage$E4PartListener.partActivated(WorkbenchPage.java:217) at org.eclipse.e4.ui.internal.workbench.PartServiceImpl$3.run(PartServiceImpl.java:253) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:45) at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.firePartActivated(PartServiceImpl.java:250) at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(PartServiceImpl.java:774) at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(PartServiceImpl.java:683) at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(PartServiceImpl.java:678) at org.eclipse.ui.internal.WorkbenchPage.activate(WorkbenchPage.java:961) at org.eclipse.papyrus.infra.ui.editor.CoreMultiDiagramEditor.doReload(CoreMultiDiagramEditor.java:976) at org.eclipse.papyrus.infra.ui.editor.CoreMultiDiagramEditor.access$4(CoreMultiDiagramEditor.java:952) at org.eclipse.papyrus.infra.ui.editor.CoreMultiDiagramEditor$DeferredReload.reloadEditor(CoreMultiDiagramEditor.java:1242) at org.eclipse.papyrus.infra.ui.editor.CoreMultiDiagramEditor$DeferredReload.reload(CoreMultiDiagramEditor.java:1216) at org.eclipse.papyrus.infra.ui.editor.CoreMultiDiagramEditor$4.reloadEditor(CoreMultiDiagramEditor.java:850) at org.eclipse.papyrus.infra.ui.services.ResourceUpdateService$4.runInUIThread(ResourceUpdateService.java:161) at org.eclipse.ui.progress.UIJob.lambda$0(UIJob.java:95) at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40) at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:185) at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4928) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4449) at org.eclipse.ui.internal.dialogs.EventLoopProgressMonitor.runEventLoop(EventLoopProgressMonitor.java:127) at org.eclipse.ui.internal.dialogs.EventLoopProgressMonitor.worked(EventLoopProgressMonitor.java:188) at org.eclipse.papyrus.infra.core.resource.ModelSet.save(ModelSet.java:777) at org.eclipse.papyrus.infra.ui.lifecycleevents.SaveAndDirtyService.doSave(SaveAndDirtyService.java:289) at org.eclipse.papyrus.infra.ui.editor.CoreMultiDiagramEditor.doSave(CoreMultiDiagramEditor.java:997) at org.eclipse.ui.internal.SaveableHelper.lambda$0(SaveableHelper.java:156) at org.eclipse.ui.internal.SaveableHelper.lambda$3(SaveableHelper.java:271) at org.eclipse.jface.operation.ModalContext.runInCurrentThread(ModalContext.java:436) at org.eclipse.jface.operation.ModalContext.run(ModalContext.java:352) at org.eclipse.ui.internal.WorkbenchWindow.lambda$5(WorkbenchWindow.java:2375) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:72) at org.eclipse.ui.internal.WorkbenchWindow.run(WorkbenchWindow.java:2373) at org.eclipse.ui.internal.SaveableHelper.runProgressMonitorOperation(SaveableHelper.java:278) at org.eclipse.ui.internal.SaveableHelper.runProgressMonitorOperation(SaveableHelper.java:260) at org.eclipse.ui.internal.SaveableHelper.savePart(SaveableHelper.java:160) at org.eclipse.ui.internal.WorkbenchPage.saveSaveable(WorkbenchPage.java:3820) at org.eclipse.ui.internal.WorkbenchPage.saveEditor(WorkbenchPage.java:3833) at org.eclipse.ui.internal.handlers.SaveHandler.execute(SaveHandler.java:81) at org.eclipse.ui.internal.handlers.HandlerProxy.execute(HandlerProxy.java:283) at org.eclipse.ui.internal.handlers.E4HandlerProxy.execute(E4HandlerProxy.java:97) at jdk.internal.reflect.GeneratedMethodAccessor88.invoke(Unknown Source) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:58) at org.eclipse.e4.core.internal.di.InjectorImpl.invokeUsingClass(InjectorImpl.java:319) at org.eclipse.e4.core.internal.di.InjectorImpl.invoke(InjectorImpl.java:253) at org.eclipse.e4.core.contexts.ContextInjectionFactory.invoke(ContextInjectionFactory.java:173) at org.eclipse.e4.core.commands.internal.HandlerServiceHandler.execute(HandlerServiceHandler.java:156) at org.eclipse.core.commands.Command.executeWithChecks(Command.java:488) at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:487) at org.eclipse.e4.core.commands.internal.HandlerServiceImpl.executeHandler(HandlerServiceImpl.java:213) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.executeCommand(KeyBindingDispatcher.java:308) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.press(KeyBindingDispatcher.java:584) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.processKeyEvent(KeyBindingDispatcher.java:653) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.filterKeySequenceBindings(KeyBindingDispatcher.java:443) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.access$2(KeyBindingDispatcher.java:386) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher$KeyDownFilter.handleEvent(KeyBindingDispatcher.java:96) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89) at org.eclipse.swt.widgets.Display.filterEvent(Display.java:1846) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1422) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1449) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1432) at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:1473) at org.eclipse.swt.widgets.Widget.gtk_key_press_event(Widget.java:837) at org.eclipse.swt.widgets.Control.gtk_key_press_event(Control.java:3933) at org.eclipse.swt.widgets.Composite.gtk_key_press_event(Composite.java:855) at org.eclipse.swt.widgets.Widget.windowProc(Widget.java:2257) at org.eclipse.swt.widgets.Control.windowProc(Control.java:6659) at org.eclipse.swt.widgets.Display.windowProc(Display.java:5919) at org.eclipse.swt.internal.gtk.GTK.gtk_main_do_event(Native Method) at org.eclipse.swt.widgets.Display.eventProc(Display.java:1486) at org.eclipse.swt.internal.gtk.OS.g_main_context_iteration(Native Method) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4444) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1158) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1047) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155) at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:658) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:557) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:154) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:150) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:137) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:107) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:401) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:657) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:594) at org.eclipse.equinox.launcher.Main.run(Main.java:1447) at org.eclipse.equinox.launcher.Main.main(Main.java:1420)