Community
Participate
Working Groups
Created attachment 267112 [details] Stack trace of the NPE occuring in the DeferredLayoutCommand Adding and saving a semantic element from a Xtext editor when a corresponding Sirius diagram editor is open leads to a NPE when Sirius is updating the layout of the diagram involving the added element. The parent or root EditPart (in AbstractEditPart) is null, most likely because the diagram is open but not active when saving from the Xtext editor. Reproduce by adding and saving a semantic element from a Xtext editor while an open diagram in Sirius is rendering the element. I have not been able to reproduce the error when adding a semantic element from an EMF editor.
Hello, Can you provide a sample with the XText editor so we can reproduce the issue? We are currently busy working on the M6 release, I do not know when we will be able to look into this issue. Regards, Steve
(In reply to Steve Monnier from comment #1) > Hello, > > Can you provide a sample with the XText editor so we can reproduce the issue? > > We are currently busy working on the M6 release, I do not know when we will > be able to look into this issue. > > Regards, > Steve Hello, I've created a Xtext project from the "Five simple steps to your JVM language" tutorial and added a minimal specification and representation bundle. You can clone or fork it from: https://github.com/eirikg/XbaseSiriusIntegration There is readme.txt file at the root of the org.example.domainmodel bundle project descibing how to reproduce. Hope you can use it to verify the exception.
Hello Eirik, Thanks for your sample, I have been able to reproduce your issue. I have a fix that I just pushed to Gerrit (https://git.eclipse.org/r/#/c/94761/). We will try to review and integrated it as soon as possible. Regards, Steve
New Gerrit change created: https://git.eclipse.org/r/94761
Hello, The fix is not as simple as the previous gerrit which causes some regressions. Unfortunately, we do not know when we will look into it as we are currently focused on the Sirius 5.0 release. Regards, Steve
Hello, I have the same problem (identical stack trace). I tested on last version of Sirius 4 and Sirius 5. I tested the bug fix, it resolve the NPE. When do you plan to apply a correction ? Best, Jonathan
Hello Jonathan I am currently working on this bug. The fix looks correct and I expect it to be available on Sirius 6.0.0 soon.
This fix consists in executing the code instead of calling a Display.asyncExec to execute the code. I am not able to guarantee that SiriusCanonicalLayoutCommand is called in a context where it would be executed on the UIThread. So, a safer fix would be that the code of SiriusCanonicalLayoutCommand.doExecute would be done on UIThread and with syncExec, calling Display.syncExec Concerning the junit test, i was not able to reproduce the issue with a non xtext context. I tried to do an external change of the dmodel(mode semantic) with a emf reflexive editor but in that case the issue does not occur. The difference I can see is that, in that case, the JvmGenericType object are not created. If the change is done in an xtext editor, that JvmGenericType are created by the Xtext builder. So the conclusion is that, in the case of the issue, the SiriusCanonicalLayoutCommand.doExecute content calls code in asyncExec which let the xtext builder create JvmGenericType before the job is called which probably do indirect effects on the EditPart model of the sirius editor. So for now I will not provide a junit test fot that fix.
New Gerrit change created: https://git.eclipse.org/r/121576
New Gerrit change created: https://git.eclipse.org/r/121585
Could SiriusCanonicalLayoutCommand be a precommit listener? SiriusCanonicalLayoutCommand uses, at the end, the gmf DeferredLayoutCommand which requires the EditPart to be created. The editPart are created only in the post commit(too late). That's why SiriusCanonicalLayoutCommand is called by a OperationHistoryListener after post the commit. So it can not a precommit listener and then the attempt on commit https://git.eclipse.org/r/121576 is hopeless. It would require some changes: - do without EditPart - make sure that layout computation on NodeContainer or Compartment (particularly in autosize) does not require some Figure to be created. That's a big work. Layouting : SiriusCanonicalLayoutCommand.doExecute() implementation alternatives 1 using Display.async is at the origin of this ticket and causes NPE 2 using Display.sync make some tests to fail in dead lock before refresh that is called from swtbot code on a no UIThread. We could accept making the supposition that, on a runtime(not from test), refresh or other kind of code that require the UI is, in fact, called on UIThread. We can hon 3 calling the code directly is not a good idea because in some particular case of layout with container in autosize when need some draw2d figure to layout properly.
I went deeper in proposal 2 (Display.syncExec) To avoid dead lock, it is possible to avoid dead lock calling refreshDiagram from UI Thread and not directly from swt test code. But unfortunately, the refresh code is also called on no UIThread by Sirius code: when click on refresh on a diagram, we are on UIThread using ProgressMonitorDialog. We call monitorDialog.run(true, false, new RefreshRunnableWithProgress(editParts)). The first boolean indicates that it is forked, that is is executed on non UI Thread. RefreshRunnableWithProgress will do the refresh and then the SiriusCanonicalLayoutCommand is called. then Display.syncExec is called then Lock.uiAcquire in EMFT that require the UI Dead lock : ProgressMonitorDialog waits for RefreshRunnableWithProgress to finish and RefreshRunnableWithProgress wait for UIThread.
I'll try to have one last look if I can get the time in the next few days, but so far all our attempts to integrate a fix have very bad side effects on other scenarios (e.g. deadlocks), and our current understanding is that fixing this correctly would require deep changes. Most probably the ticket will be moved to 6.1 (planned for this autumn). Hopefully the analysis we've now done will allow us to make a concrete plan to fix the underlying issues instead of moving it again next time.
Moving to 6.1, sorry. It doesn't seem like we'll be able to have another look before the 6.0 release (which is coming soon).
I have also encountered this problem recently while trying out the Xtext-Example statemachine project available under https://github.com/ObeoNetwork/Xtext-Sirius-integration. Stacktrace: null org.eclipse.ui Error Tue Nov 27 08:36:16 CET 2018 Unhandled event loop exception java.lang.NullPointerException at org.eclipse.gef.editparts.AbstractEditPart.createChild(AbstractEditPart.java:269) at org.eclipse.gef.editparts.AbstractEditPart.refreshChildren(AbstractEditPart.java:780) at org.eclipse.gef.editparts.AbstractEditPart.refresh(AbstractEditPart.java:726) at org.eclipse.gef.editparts.AbstractGraphicalEditPart.refresh(AbstractGraphicalEditPart.java:644) at org.eclipse.gmf.runtime.diagram.ui.editparts.GraphicalEditPart.access$3(GraphicalEditPart.java:1) at org.eclipse.gmf.runtime.diagram.ui.editparts.GraphicalEditPart$3.run(GraphicalEditPart.java:861) at org.eclipse.emf.transaction.impl.TransactionalEditingDomainImpl.runExclusive(TransactionalEditingDomainImpl.java:328) at org.eclipse.gmf.runtime.diagram.ui.editparts.GraphicalEditPart.refresh(GraphicalEditPart.java:851) at org.eclipse.gmf.runtime.diagram.ui.commands.DeferredLayoutCommand$1.run(DeferredLayoutCommand.java:146) at org.eclipse.gmf.runtime.diagram.ui.util.EditPartUtil.synchronizeRunnableToMainThread(EditPartUtil.java:106) at org.eclipse.gmf.runtime.diagram.ui.commands.DeferredLayoutCommand.doExecuteWithResult(DeferredLayoutCommand.java:186) at org.eclipse.gmf.runtime.emf.commands.core.command.AbstractTransactionalCommand.doExecute(AbstractTransactionalCommand.java:247) at org.eclipse.emf.workspace.AbstractEMFOperation.execute(AbstractEMFOperation.java:150) at org.eclipse.gmf.runtime.diagram.ui.commands.ICommandProxy.execute(ICommandProxy.java:68) at org.eclipse.gef.commands.CompoundCommand.execute(CompoundCommand.java:129) at org.eclipse.sirius.diagram.ui.internal.refresh.layout.SiriusCanonicalLayoutCommand.executeLayoutDueToExternalChanges(SiriusCanonicalLayoutCommand.java:109) at org.eclipse.sirius.diagram.ui.internal.refresh.layout.SiriusCanonicalLayoutCommand.access$3(SiriusCanonicalLayoutCommand.java:106) at org.eclipse.sirius.diagram.ui.internal.refresh.layout.SiriusCanonicalLayoutCommand$1.run(SiriusCanonicalLayoutCommand.java:92) 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:3933) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3564) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1173) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1062) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:156) at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:628) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:563) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:151) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:155) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:199) 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:391) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:246) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:659) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:595) at org.eclipse.equinox.launcher.Main.run(Main.java:1501) at org.eclipse.equinox.launcher.Main.main(Main.java:1474)
Created attachment 276714 [details] Screencast reproducing the problem
