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

Bug 352860

Summary: NullPointerException when closing using system menu item (SWT.ID_QUIT)
Product: [Eclipse Project] Platform Reporter: Thomas Singer <eclipse>
Component: SWTAssignee: Lakshmi P Shanmugam <lshanmug>
Status: RESOLVED FIXED QA Contact: Silenio Quarti <Silenio_Quarti>
Severity: normal    
Priority: P3 CC: eclipse.felipe, lshanmug
Version: 3.7   
Target Milestone: 3.8 M1   
Hardware: PC   
OS: Mac OS X   
Whiteboard:
Attachments:
Description Flags
Sample to reproduce the bug
none
stacktrace
none
patch none

Description Thomas Singer CLA 2011-07-22 09:35:05 EDT 
Build Identifier: 3.7.0.v3735b

1. Launch the attached test case (OS X 10.6 or 10.7)
2. Press Cmd+Q
3. The attached stacktrace occurs

Reproducible: Always
Comment 1 Thomas Singer CLA 2011-07-22 09:36:18 EDT 
Created attachment 200183 [details]
Sample to reproduce the bug
Comment 2 Thomas Singer CLA 2011-07-22 09:38:39 EDT 
Created attachment 200184 [details]
stacktrace
Comment 3 Thomas Singer CLA 2011-07-22 09:39:23 EDT 
The sample has been launched with the VM options "-d32 -XstartOnFirstThread".
Comment 4 Lakshmi P Shanmugam CLA 2011-07-25 08:59:05 EDT 
Created attachment 200276 [details]
patch

The exception is caused because the default menu action is invoked after the display has been disposed in the Selection Listener. It can be prevented in the code by setting event.doit = false in the Listener so that the default action is not called.

Patch adds a check to see if the display is disposed. Silenio, can you please review?
Comment 5 Felipe Heidrich CLA 2011-07-25 12:34:42 EDT 
(In reply to comment #4)
> Patch adds a check to see if the display is disposed. Silenio, can you please
> review?

instead of:
	// display may be disposed at this point 
	if (display == null || display.isDisposed()) return; 

would the usuall check work ?
	// widget could be disposed at this point
	if (isDisposed ()) return;

(note, you need to release the code to GIT, if you release to CVS the change will be lost).
Comment 6 Lakshmi P Shanmugam CLA 2011-07-26 06:49:33 EDT 
(In reply to comment #5)
> (In reply to comment #4)
> > Patch adds a check to see if the display is disposed. Silenio, can you please
> > review?
> 
> instead of:
>     // display may be disposed at this point 
>     if (display == null || display.isDisposed()) return; 
> 
> would the usuall check work ?
>     // widget could be disposed at this point
>     if (isDisposed ()) return;
> 
> (note, you need to release the code to GIT, if you release to CVS the change
> will be lost).
Yes, the above check works. I wonder why I didn't do that... :(
Comment 7 Felipe Heidrich CLA 2011-07-26 12:14:10 EDT 
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Patch adds a check to see if the display is disposed. Silenio, can you please
> > > review?
> > 
> > instead of:
> >     // display may be disposed at this point 
> >     if (display == null || display.isDisposed()) return; 
> > 
> > would the usuall check work ?
> >     // widget could be disposed at this point
> >     if (isDisposed ()) return;
> > 
> > (note, you need to release the code to GIT, if you release to CVS the change
> > will be lost).
> Yes, the above check works. I wonder why I didn't do that... :(

good, please release the code if you haven't already done so.