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

Bug 356935

Summary:

SWT's implementation for UiSynchronize doesn't check for Display device disposal

Product:

z_Archived

Reporter:

Simon Chemouil <eclipse>

Component:

Assignee:

Simon Chemouil <eclipse>

Status:

ASSIGNED ---

QA Contact:

Severity:

normal

Priority:

CC:

remy.suen

Version:

unspecified

Target Milestone:

---

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Patch with Display#isDisposed() check	none
Patch with Display#isDisposed() check	none

Description Simon Chemouil

2011-09-07 09:39:44 EDT

UiSynchronize is a wrapper class aimed at decoupling the UI framework API from E4 workbench and provide two methods (syncExec & asyncExec).

SWT's implementation for UiSynchronize doesn't check for Display device disposal and there is no way in UISynchronize to check if its methods are legal to call (ie, no "boolean isAlive()" method).

UiSynchronize is then used at different places to schedule runnables to the UI thread. UI Event Topics messages are among these events.

If closing your application sends an event that is listened by an @UiEventTopic annotated method, you will get a SWT error "Device is disposed".

I provide a patch to check if the display is disposed before calling either syncExec or asyncExec.


Stacktrace with current code:
org.eclipse.swt.SWTException: Device is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4282)
	at org.eclipse.swt.SWT.error(SWT.java:4197)
	at org.eclipse.swt.SWT.error(SWT.java:4168)
	at org.eclipse.swt.widgets.Display.error(Display.java:1210)
	at org.eclipse.swt.widgets.Display.syncExec(Display.java:4321)
	at org.eclipse.e4.ui.internal.workbench.swt.E4Application$1.syncExec(E4Application.java:182)
	at org.eclipse.e4.ui.services.internal.events.UIEventHandler.handleEvent(UIEventHandler.java:38)
	at org.eclipse.equinox.internal.event.EventHandlerWrapper.handleEvent(EventHandlerWrapper.java:197)
	at org.eclipse.equinox.internal.event.EventHandlerTracker.dispatchEvent(EventHandlerTracker.java:197)

Comment 1 Simon Chemouil

2011-09-07 09:41:21 EDT

Created attachment 202895 [details]
Patch with Display#isDisposed() check

Works for me, +1 or -1 so I can update and/or commit.

Comment 2 Simon Chemouil

2011-09-07 09:57:03 EDT

Created attachment 202900 [details]
Patch with Display#isDisposed() check

Added missing copyright entry.

Comment 3 Remy Suen

2011-09-07 09:58:11 EDT

(In reply to comment #0)
> If closing your application sends an event that is listened by an @UiEventTopic
> annotated method, you will get a SWT error "Device is disposed".

I guess this is not an object that's a part of the model? I would expect objects that are part of the model to be disposed (and hence not be injected again).

Comment 4 Simon Chemouil

2011-09-07 10:08:01 EDT

(In reply to comment #3)
> I guess this is not an object that's a part of the model? I would expect
> objects that are part of the model to be disposed (and hence not be injected
> again).

It's not in the model per se but it's added to the applicationContext and injected at least to UiEventHandler. And it's not "uninjected" when the display is disposed. It's useless to schedule runnables if the display is disposed, but I agree that it would be nice to find a way "out" from doing the check.

Comment 5 Remy Suen

2011-09-08 11:10:56 EDT

Simon, I checked what we do in DisplayRealm and it seems to match what your code does.

For the syncExec(Runnable) case, can you check if the current thread's display is actually the display we have and if it is the same to just run the runnable? Otherwise, just queue it as an asynchronous call (and it will be ignored by the disposal check).