Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 352315 - NPE in BreakpointManager.removeBreakpointListener() during shutdown
Summary: NPE in BreakpointManager.removeBreakpointListener() during shutdown
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 3.8 M2   Edit
Assignee: Pawel Piech CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-18 04:41 EDT by Mirko Seifert CLA
Modified: 2011-09-13 17:19 EDT (History)
3 users (show)

See Also:
Michael_Rennie: review+

Attachments
Proposed simple fix. (1.31 KB, patch)
2011-07-20 12:31 EDT, Pawel Piech 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 Mirko Seifert CLA 2011-07-18 04:41:43 EDT 
Build Identifier: I20110613-1736

If removeBreakpointListener() is called after the BreakpointManager has been shut down by the DebugPlugin, a NPE is thrown, because some fields in BreakpointManager are set to null in shutdown(). 

The methods to add/remove listeners should either check whether the field are null and do nothing in this case, or the method DebugPlugin.isShuttingDown() should be public to allow clients to check whether a shut down is currently going on. In the latter case, client can avoid calls to the add/remove listener methods.

Reproducible: Always
Comment 1 Pawel Piech CLA 2011-07-20 12:31:06 EDT 
Created attachment 200026 [details]
Proposed simple fix.
Comment 2 Pawel Piech CLA 2011-07-20 12:33:42 EDT 
It seems that the most correct thing to do would be to add another synchronization object that is set at shutdown and checked before every listener add/remove.  This seems like a lot of fluff for the edge case.  Instead I think we can just clear the lists instead of setting them to null.

Mike, do you agree?
Comment 3 Michael Rennie CLA 2011-08-08 12:26:28 EDT 
Makes sense.
Comment 4 Michael Rennie CLA 2011-08-08 13:54:48 EDT 
Applied a version of the patch that does not have a compile error in it to HEAD.
Comment 5 Curtis Windatt CLA 2011-09-13 17:19:13 EDT 
Verified in I20110912-2126