| Summary: | IllegalStateException from WorkbenchMenuService when extension registry changed | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [RT] RAP | Reporter: | Nicholas Hinds <hindsn> | ||||||||
| Component: | Workbench | Assignee: | Project Inbox <rap-inbox> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | dubauski_psl, ivan | ||||||||
| Version: | 1.3 | ||||||||||
| Target Milestone: | 2.2 M1 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Linux | ||||||||||
| Whiteboard: | sr211 | ||||||||||
| Attachments: |
|
||||||||||
Created attachment 193456 [details]
Proposed fix
Attached a proposed patch to fix this bug in WorkbenchMenuService - instead of calling PlatformUI.getWorkbench().getDisplay(), keep a reference to the correct thread and look up the display from Display.findDisplay(...).
Nicholas, probably replacing the PlatformUI.getWorkbench().getDisplay() with Display#getCurrent() will fix the problem too. Could you confirm this? Based on a quick look, Display.getCurrent() will fail because like getWorkbench(), it calls down to ContextProvider. It won't throw an exception, since it calls hasContext() instead of getContext(), but it will return null since the work is happening in the background ExtensionRegistry event thread which has no associated context. I can try to test it with Display.getCurrent() if you like. (In reply to comment #3) > I can try to test it with Display.getCurrent() if you like. I see your point, but an additional test will not hurt. Thanks. Sorry, I forgot this was waiting on my input. I have verified that replacing PlatformUI.getWorkbench().getDisplay() with Display.getCurrent() throws a NullPointerException, since Display.getCurrent() returns null. Has there been any progress in getting this fixed? FWIW under RAP 2.0 the exception is still present but looks slightly different: Thread [Worker-1] (Suspended (exception IllegalStateException)) ContextProvider.getContext() line: 98 ContextProvider.getUISession() line: 186 SingletonUtil.getSessionInstance(Class<T>) line: 54 Workbench.getInstance() line: 652 PlatformUI.getWorkbench() line: 94 WorkbenchMenuService$1.registryChanged(IRegistryChangeEvent) line: 266 ExtensionRegistry$2.run() line: 922 SafeRunner.run(ISafeRunnable) line: 42 ExtensionRegistry.processChangeEvent(Object[], Map) line: 920 RegistryStrategy.processChangeEvent(Object[], Map, Object) line: 260 ExtensionEventDispatcherJob.run(IProgressMonitor) line: 50 Worker.run() line: 53 Our application relies on several bundles being installed dynamically and loading some of them triggers registry change events. All these errors can't be ignored as they are reported in "Mutliple problems have occurred" modal dialog that pops on top of the application. See the attachment. We would be very interested in getting this fixed ASAP. Created attachment 231053 [details]
Screenshot of the problem dialog
Is there anyone monitoring this? Please give an update on questions in comments 6 and 7. Thank you. Can you try the attached patch in comment#1? Does it fix your problem? Given the current state of the code (and the existence of IWorkbenchLocationService) it may be more appropriate to simply make 'wls' final and replace 'PlatformUI.getWorkbench()' with 'wls.getWorkbench()' on lines 256 and 266 of WorkbenchMenuService.java, respectively. (In reply to comment #9) > Can you try the attached patch in comment#1? Does it fix your problem? That patch didn't fix the problem. I still got the same number of IllegalStateExceptions java.lang.IllegalStateException: No context available outside of the request processing. at org.eclipse.rap.rwt.internal.service.ContextProvider.getContext(ContextProvider.java:98) at org.eclipse.rap.rwt.internal.application.ApplicationContextUtil.getInstance(ApplicationContextUtil.java:73) at org.eclipse.rap.rwt.internal.service.ContextProvider.getApplicationContext(ContextProvider.java:206) at org.eclipse.swt.widgets.Display.getDisplays(Display.java:2204) at org.eclipse.swt.widgets.Display.findDisplay(Display.java:911) at org.eclipse.ui.internal.menus.WorkbenchMenuService$1.registryChanged(WorkbenchMenuService.java:267) at org.eclipse.core.internal.registry.ExtensionRegistry$2.run(ExtensionRegistry.java:922) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.core.internal.registry.ExtensionRegistry.processChangeEvent(ExtensionRegistry.java:920) at org.eclipse.core.runtime.spi.RegistryStrategy.processChangeEvent(RegistryStrategy.java:260) at org.eclipse.core.internal.registry.osgi.ExtensionEventDispatcherJob.run(ExtensionEventDispatcherJob.java:50) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:53) (In reply to comment #10) > Given the current state of the code (and the existence of > IWorkbenchLocationService) it may be more appropriate to simply make 'wls' > final and replace 'PlatformUI.getWorkbench()' with 'wls.getWorkbench()' on > lines 256 and 266 of WorkbenchMenuService.java, respectively. : Thank you Nicholas, this actually worked! Ivan, is there a plan to get this fix into 2.1 release of RAP? (In reply to comment #12) > (In reply to comment #10) > > Given the current state of the code (and the existence of > > IWorkbenchLocationService) it may be more appropriate to simply make 'wls' > > final and replace 'PlatformUI.getWorkbench()' with 'wls.getWorkbench()' on > > lines 256 and 266 of WorkbenchMenuService.java, respectively. > : > Thank you Nicholas, this actually worked! > > Ivan, is there a plan to get this fix into 2.1 release of RAP? Unfortunately, it's too late for 2.1 release. But we could consider it for the first service release. Fixed in master as suggested in comment#10 with commit ee82ec28966550f9b6971e8acf77c8e0392d9e34. The patch looks pretty safe, +1 for 2.1.1 Backported to 2.1-maintenance branch with commit ce18cefe151b344fbf52c2b11f5ea6b141ecb91f. |
Created attachment 193455 [details] IllegalArgumentException stacktrace Build: 1.3.2.20110216-1023 Steps to reproduce: * Open a RAP workbench * Install & start a bundle which causes a RegistryChangeEvent Expected outcome: No errors Actual outcome: Stacktrace attached. The exception seems to occur because the event is delivered in a background thread, but PlatformUI.getWorkbench() tries to access the (thread-local) context to get the applicable workbench. This is very similar to #290920, but for a different component.