Bug 569067 - Layout Spy launched from Ctrl-3 dialog often fails with !MESSAGE Error executing 'org.eclipse.tools.layout.spy.commands.layoutSpyCommand': java.lang.IllegalArgumentException: Argument not valid
Summary: Layout Spy launched from Ctrl-3 dialog often fails with !MESSAGE Error execut...
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.18   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 4.19 M3   Edit
Assignee: Jonah Graham CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-23 10:24 EST by Jonah Graham CLA
Modified: 2021-01-09 10:55 EST (History)
5 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jonah Graham CLA 2020-11-23 10:24:26 EST 
I get this exception often when launching layout spy from Ctrl-3 dialog. I think that the recentish changes to how that dialog works causes the regression as it created a shell, where in the past it didn't (??? not sure if that is true).

Anyway, the problem is that org.eclipse.tools.layout.spy.internal.handlers.LayoutSpyHandler.execute(ExecutionEvent) calls HandlerUtil.getActiveShell(event) but that returns the Find Actions shell, which is disposed by the time the handler is actually run, which leads to the exception below.

Note this does not fail all the time, when it does not fail the active shell returned is the (expected) main Eclipse IDE window. 

Therefore there appears to be a race condition in which org.eclipse.ui.ISources.ACTIVE_SHELL_NAME has for a short time a disposed shell in it.

Because org.eclipse.ui.ISources.ACTIVE_SHELL_NAME can have a disposed shell in it, this seems likely to affect others than just the layout spy. While we can fix this locally at layout spy by looking for disposed shell before proceeding, should it be that org.eclipse.ui.ISources.ACTIVE_SHELL_NAME can have a disposed shell?

Full stack trace:


!ENTRY org.eclipse.ui 4 0 2020-11-23 10:17:51.607
!MESSAGE Error executing 'org.eclipse.tools.layout.spy.commands.layoutSpyCommand': java.lang.IllegalArgumentException: Argument not valid
!STACK 0
org.eclipse.core.commands.ExecutionException: Error executing 'org.eclipse.tools.layout.spy.commands.layoutSpyCommand': java.lang.IllegalArgumentException: Argument not valid
	at org.eclipse.e4.core.commands.internal.HandlerServiceHandler.execute(HandlerServiceHandler.java:170)
	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.ui.internal.handlers.LegacyHandlerService.executeCommandInContext(LegacyHandlerService.java:440)
	at org.eclipse.ui.internal.quickaccess.providers.CommandElement.execute(CommandElement.java:61)
	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:4994)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4500)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1157)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1046)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:644)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:551)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:156)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
	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:653)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:590)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1461)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1434)
Caused by: org.eclipse.e4.core.di.InjectionException: java.lang.IllegalArgumentException: Argument not valid
	at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:68)
	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)
	... 31 more
Caused by: java.lang.IllegalArgumentException: Argument not valid
	at org.eclipse.swt.SWT.error(SWT.java:4850)
	at org.eclipse.swt.SWT.error(SWT.java:4784)
	at org.eclipse.swt.SWT.error(SWT.java:4755)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:555)
	at org.eclipse.swt.widgets.Shell.<init>(Shell.java:275)
	at org.eclipse.swt.widgets.Shell.<init>(Shell.java:369)
	at org.eclipse.tools.layout.spy.internal.dialogs.LayoutSpyDialog.<init>(LayoutSpyDialog.java:141)
	at org.eclipse.tools.layout.spy.internal.handlers.LayoutSpyHandler.execute(LayoutSpyHandler.java:35)
	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.GeneratedMethodAccessor84.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)
	... 35 more
Comment 1 Jonah Graham CLA 2020-11-23 10:24:59 EST 
I can try to fix this - but in the meantime if someone has another way of launching the Layout Spy as a workaround I would appreciate it.
Comment 2 Andrey Loskutov CLA 2020-11-23 10:36:15 EST 
See bug 568837.

Ctrl + Shift + F1 is your friend.

*** This bug has been marked as a duplicate of bug 568837 ***
Comment 3 Jonah Graham CLA 2020-11-23 10:54:44 EST 
(In reply to Andrey Loskutov from comment #2)
> See bug 568837.
> 
> Ctrl + Shift + F1 is your friend.
> 
> *** This bug has been marked as a duplicate of bug 568837 ***

That ^^^ bug is closed/invalid. Is an exception like this is not a valid bug? I suppose it may not be if the expectation is that the shell to inspect is the one that is now closed, but even in that case an exception is a bit harsh.

Also, Ctrl + Shift + F1 is not working for me, but I haven't figure out why. It was enough hint for me to find that Ctrl+Alt+Shift+F9 does work (thanks to -> https://www.vogella.com/tutorials/EclipsePlatformDevelopment/article.html)
Comment 4 Andrey Loskutov CLA 2020-11-23 11:07:55 EST 
(In reply to Jonah Graham from comment #3)
> > *** This bug has been marked as a duplicate of bug 568837 ***
> 
> That ^^^ bug is closed/invalid. Is an exception like this is not a valid
> bug? I suppose it may not be if the expectation is that the shell to inspect
> is the one that is now closed, but even in that case an exception is a bit
> harsh.

See bug 568837 comment 9. The original reported decided to close bug 568837. If you think it is worth fixing, feel free to remove the duplicate resolution and reopen this one.
Comment 5 Jonah Graham CLA 2020-11-23 11:11:14 EST 
(In reply to Andrey Loskutov from comment #4)
> If you think it is worth fixing, feel free to remove the duplicate
> resolution and reopen this one.

OK, thanks for confirming. I do (and have done so).
Comment 6 Lars Vogel CLA 2020-11-23 11:15:06 EST 
(In reply to Jonah Graham from comment #5)
> (In reply to Andrey Loskutov from comment #4)
> > If you think it is worth fixing, feel free to remove the duplicate
> > resolution and reopen this one.
> 
> OK, thanks for confirming. I do (and have done so).

Great, thanks
Comment 7 Rolf Theunissen CLA 2020-11-23 13:22:51 EST 
Seems that the root cause is already identified in Bug 220260, currently the code lives in WorkbenchSourceProvider#updateActiveShell.
Comment 8 Jonah Graham CLA 2020-11-23 13:58:40 EST 
(In reply to Rolf Theunissen from comment #7)
> Seems that the root cause is already identified in Bug 220260, currently the
> code lives in WorkbenchSourceProvider#updateActiveShell.

Thanks Rolf - that bug looks exactly correct. Being a 12 year old bug, it may be that it is too hard to solve, so I have this bug to address the immediate need of working around Bug 220260 locally.
Comment 9 Eclipse Genie CLA 2020-11-24 12:20:48 EST 
New Gerrit change created: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/172772