Bug 338221

Comment 1 Paul Webster 2011-02-25 09:23:46 EST

Prakash, does this mean we just haven't ported the SWT.Close changes from 3.7 to 4.x?

PW

Comment 2 DJ Houghton 2011-05-02 09:54:10 EDT

build I20110428-0200

I noticed with builds last week I was prompted before exit. But today when I exited I was prompted *after* the workspace closed. So I hit "cancel" but Eclipse still exited anyway.

Comment 3 DJ Houghton 2011-05-02 10:59:03 EDT

Ok, this happened again with build I20110501-0200.
This time I hit "ok" and it showed the "saving workspace" progress dialog again. 
The spewed this error:


java.lang.IllegalStateException: The service has been unregistered
	at org.eclipse.osgi.internal.serviceregistry.ServiceRegistrationImpl.unregister(ServiceRegistrationImpl.java:209)
	at org.eclipse.ui.internal.Workbench.shutdown(Workbench.java:2599)
	at org.eclipse.ui.internal.Workbench.busyClose(Workbench.java:1052)
	at org.eclipse.ui.internal.Workbench.access$12(Workbench.java:947)
	at org.eclipse.ui.internal.Workbench$15.run(Workbench.java:1116)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
	at org.eclipse.ui.internal.Workbench.close(Workbench.java:1114)
	at org.eclipse.ui.internal.Workbench.close(Workbench.java:1087)
	at org.eclipse.ui.internal.Workbench$50.handleEvent(Workbench.java:2389)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4117)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4110)
	at org.eclipse.swt.widgets.Display.applicationProc(Display.java:5130)
	at org.eclipse.swt.internal.cocoa.OS.objc_msgSend_bool(Native Method)
	at org.eclipse.swt.internal.cocoa.NSApplication.sendAction(NSApplication.java:124)
	at org.eclipse.swt.widgets.MenuItem.sendSelection(MenuItem.java:568)
	at org.eclipse.swt.widgets.Display.windowProc(Display.java:5304)
	at org.eclipse.swt.internal.cocoa.OS.objc_msgSendSuper(Native Method)
	at org.eclipse.swt.widgets.Display.applicationSendEvent(Display.java:4974)
	at org.eclipse.swt.widgets.Display.applicationProc(Display.java:5123)
	at org.eclipse.swt.internal.cocoa.OS.objc_msgSend(Native Method)
	at org.eclipse.swt.internal.cocoa.NSApplication.sendEvent(NSApplication.java:128)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3599)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$7.run(PartRenderingEngine.java:890)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:806)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:87)

Comment 4 DJ Houghton 2011-05-04 15:33:48 EDT

Increasing severity. This happens to me quite regularly now. If I hit "Cancel" then the prompt goes away and if I hit "OK" I get an error. Either way the user is confused and has no idea if their workspace was saved successfully and if everything is in a good state.

Comment 5 Praveen 2011-05-09 09:56:39 EDT

I tried investigating into this. When command-Q is pressed, QuitHandler is invoked prior to SWT.Close event of the Workbench window. The reason for this - surprisingly, we are getting a MenuItem selection event (when command-Q is pressed) prior to SWT.Close event. Thus the framework invokes the file.exit handler (QuitHandler) in order to execute the command.
So, though we get a chance to prompt for the exit, it couldn't stop the shutdown as QuitHandler has already closed everything.
I couldn't figure out why did the menuItem selection event invoked for command-Q unlike Eclipse 3.x. Also, I couldn't find who creates the *SWT MenuItem Widget (Quit Eclipse)*, but surprisingly the same is resolved when the menu-item selection event is processed.

Comment 6 Brian de Alwis 2011-05-12 13:37:56 EDT

Since DJ's on a Mac, org.eclipse.e4.ui.workbench.renderers.swt.cocoa causes the Cmd-Q / Quit item to trigger the "org.eclipse.ui.file.exit" command.  I guess that's a different code path than what happens with Window > Close.

Comment 7 DJ Houghton 2011-05-12 13:40:26 EDT

Also note this happens when you do Eclipse -> Quit Eclipse as well.

Comment 8 Brian de Alwis 2011-05-13 13:58:41 EDT

Created attachment 195612 [details]
Patch to fix issue

I figured out the problem: I changed CocoaUIHandler to use the new SWT APIs for handling QUIT, PREFERENCES, and ABOUT.  These APIs require setting 'e.doit = false' to prevent the event from being handled by the OS, which I failed to do.

(The line numbers on this patch may vary from what's currently in HEAD — my file included the changes in 345494.)

Comment 9 Brian de Alwis 2011-05-13 13:59:16 EDT

Paul, would you mind giving this a look too?

Comment 10 Brian de Alwis 2011-05-18 12:31:49 EDT

Just two quick notes on the patch: large parts of the diff are simply indenting changes.   In addition to the menu-exit fix (found at the very top), the patch includes two changes to avoid theoretical but unlikely-to-occur events.

In runAction(), the action won't actually be performed if the display didn't have a menuBar.  Although though this situation is unlikely to ever occur, it's incorrect. I inverted the top-level condition in runAction() to do a fastpath return to simplify reading through the method.

runCommand() didn't have a check for that the handler service was available; very unlikely to occur, but since we're testing for the command service.  As with runAction(), I inverted a condition to do a fastpath return.

Comment 11 Paul Webster 2011-05-18 13:57:50 EDT

I think it's fine.
PW

Comment 12 Brian de Alwis 2011-05-18 14:52:22 EDT

Fix committed to HEAD