Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 352502 - [breakpoints] Toggle breakpoints type logic can select an invalid breakpoint type.
Summary: [breakpoints] Toggle breakpoints type logic can select an invalid breakpoint ...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 3.7   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 3.7.1   Edit
Assignee: Pawel Piech CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-19 15:45 EDT by Pawel Piech CLA
Modified: 2011-07-21 12:47 EDT (History)
3 users (show)

See Also:
Michael_Rennie: review+

Attachments
Proposed fix. (12.15 KB, patch)
2011-07-19 15:47 EDT, Pawel Piech CLA 		no flags Details | Diff
Test code. (3.74 KB, patch)
2011-07-19 18:16 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 Pawel Piech CLA 2011-07-19 15:45:59 EDT 
If an IToggleBreakpointsTargetFactory is registered without an enablement expression, and this factory returns a valid target ID for getDefaultToggleTarget(), then such toggle factory may cause another toggle factory to be disabled such that no breakpoints can be set.

The problem is in the logic that selects the default toggle breakpoints target.  It polls all factories that are enabled for their default toggle target ID.  This default target ID can be set as the default for a set of available IDs, even if this default ID is not one of the currently active IDs.  

This bug showed up in Wind River product with the addition of PHP breakpoint toggle factory (see bug 352492).
Comment 1 Pawel Piech CLA 2011-07-19 15:47:46 EDT 
Created attachment 199942 [details]
Proposed fix.

This fix adjusts the default toggle target selection to only consider the toggle target IDs that were returned by IToggleBreakpointsTargetFactory.getToggleTargets() calls.
Comment 2 Pawel Piech CLA 2011-07-19 15:55:10 EDT 
I committed the fix to head and 3.7.1 branch.  Mike please have a look at the changes when you have a chance.
Comment 3 Pawel Piech CLA 2011-07-19 18:16:08 EDT 
Created attachment 199949 [details]
Test code.

I used this patch to the PDA example to reproduce the bug.  Must launch with clean workspace to see the bug though.
Comment 4 Dani Megert CLA 2011-07-20 03:24:40 EDT 
(In reply to comment #2)
> I committed the fix to head and 3.7.1 branch.  Mike please have a look at the
> changes when you have a chance.

Hi Pawel, next time please wait for the review+ on the bug before putting something into a maintenance branch. Also, depending on the fix (didn't look at this one) it is often good to see how it behaves in HEAD and only if it behaves well, do the backport. Both things are there to make the maintenance stream more robust and to save you work in case the reviewer wants some tweaks to the fix or the fix causes regressions.
Comment 5 Martin Oberhuber CLA 2011-07-20 08:29:19 EDT 
CQ:WIND00288111
Comment 6 Michael Rennie CLA 2011-07-21 12:47:37 EDT 
+1 for 3.7.1

> Both things are there to make the maintenance stream
> more robust and to save you work in case the reviewer wants some tweaks to the
> fix or the fix causes regressions.

It is also common practice to keep the patches for maintenance branches to be as small as possible, for example this fix contained a lot of Javadoc changes that should not have been back-ported.