Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 352860 - NullPointerException when closing using system menu item (SWT.ID_QUIT)
Summary: NullPointerException when closing using system menu item (SWT.ID_QUIT)
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 3.7   Edit
Hardware: PC Mac OS X
: P3 normal (vote)
Target Milestone: 3.8 M1   Edit
Assignee: Lakshmi P Shanmugam CLA
QA Contact: Silenio Quarti CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-22 09:35 EDT by Thomas Singer CLA
Modified: 2011-07-27 06:29 EDT (History)
2 users (show)

See Also:

Attachments
Sample to reproduce the bug (704 bytes, application/octet-stream)
2011-07-22 09:36 EDT, Thomas Singer CLA 		no flags Details
stacktrace (1.26 KB, text/plain)
2011-07-22 09:38 EDT, Thomas Singer CLA 		no flags Details
patch (837 bytes, patch)
2011-07-25 08:59 EDT, Lakshmi P Shanmugam CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.