Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 343082

Summary: IllegalStateException from WorkbenchMenuService when extension registry changed
Product: [RT] RAP Reporter: Nicholas Hinds <hindsn>
Component: WorkbenchAssignee: 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:
Description Flags
IllegalArgumentException stacktrace
none
Proposed fix
none
Screenshot of the problem dialog none

Description Nicholas Hinds CLA 2011-04-17 22:18:06 EDT
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.
Comment 1 Nicholas Hinds CLA 2011-04-17 22:25:01 EDT
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(...).
Comment 2 Ivan Furnadjiev CLA 2011-09-21 06:30:45 EDT
Nicholas, probably replacing the PlatformUI.getWorkbench().getDisplay() with Display#getCurrent() will fix the problem too. Could you confirm this?
Comment 3 Nicholas Hinds CLA 2011-09-21 07:10:02 EDT
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.
Comment 4 Ivan Furnadjiev CLA 2011-09-21 07:16:16 EDT
(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.
Comment 5 Nicholas Hinds CLA 2012-09-03 20:06:02 EDT
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.
Comment 6 Barys Dubauski CLA 2013-05-16 01:56:35 EDT
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.
Comment 7 Barys Dubauski CLA 2013-05-16 01:57:06 EDT
Created attachment 231053 [details]
Screenshot of the problem dialog
Comment 8 Barys Dubauski CLA 2013-06-08 00:11:06 EDT
Is there anyone monitoring this? Please give an update on questions in comments 6 and 7.

Thank you.
Comment 9 Ivan Furnadjiev CLA 2013-06-08 04:51:17 EDT
Can you try the attached patch in comment#1? Does it fix your problem?
Comment 10 Nicholas Hinds CLA 2013-06-08 05:35:06 EDT
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.
Comment 11 Barys Dubauski CLA 2013-06-10 17:52:55 EDT
(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)
Comment 12 Barys Dubauski CLA 2013-06-10 18:05:29 EDT
(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?
Comment 13 Ivan Furnadjiev CLA 2013-06-11 02:23:26 EDT
(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.
Comment 14 Ivan Furnadjiev CLA 2013-06-11 02:38:01 EDT
Fixed in master as suggested in comment#10 with commit ee82ec28966550f9b6971e8acf77c8e0392d9e34.
Comment 15 Ralf Sternberg CLA 2013-08-09 04:37:53 EDT
The patch looks pretty safe, +1 for 2.1.1
Comment 16 Ivan Furnadjiev CLA 2013-08-13 04:07:41 EDT
Backported to 2.1-maintenance branch with commit ce18cefe151b344fbf52c2b11f5ea6b141ecb91f.