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

Bug 243899

Summary:

[breakpoints] MIBreakpointsManager does not handle command control shutdown correctly.

Product:

[Tools] CDT

Reporter:

Pawel Piech <pawel.1.piech>

Component:

cdt-debug-dsf-gdb

Assignee:

Francois Chouinard <fchouinard>

Status:

RESOLVED FIXED

QA Contact:

Marc Khouzam <marc.khouzam>

Severity:

normal

Priority:

CC:

cdtdoug, dd.general-inbox, pawel.1.piech

Version:

6.0

Flags:

cdtdoug: iplog-
marc.khouzam: review+

Target Milestone:

6.0

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Breakpoint install count handling patch	none
Updated fix	none

Description Pawel Piech

2008-08-12 11:57:41 EDT

This bug resulted from an examination of an exception in bug 240507 comment#8.  It appears that handling of the comman control shutdown, which calls MIBreakpointsManager.shutdown() results in an ArrayStoreException.  

It is not obvious what the intention behind the terminate() method is, so the call to this method is commented out for now.

Comment 1 Francois Chouinard

2009-01-28 13:31:29 EST

Created attachment 124060 [details]
Breakpoint install count handling patch

The intention is to keep the breakpoint install count attribute in sync with reality. This attribute is:
- incremented each time the breakpoint is installed
- decremented each time the breakpoint is removed
- cleared when MIBreakpointsManager terminates

If the count is not cleared, it just keeps incrementing every time a debug session is started. We reset this attribute because we assume that terminating an MI debug session also means that the breakpoints will be removed.


Anyway, the problem was introduced when the fPlatformBPs was restructured to become a map of execution contexts to a set of breakpoints. This part of the code was not kept in sync.

The attached patch takes care of this.

Comment 2 Francois Chouinard

2009-01-28 13:35:02 EST

Patch was committed.

Comment 3 Marc Khouzam

2009-01-29 10:33:25 EST

Looks good.  One comment

    	for (IBreakpointsTargetDMContext ctx : fPlatformBPs.keySet()) {
            clearBreakpointStatus(fPlatformBPs.get(ctx).keySet().toArray(new ICBreakpoint[fPlatformBPs.size()]));
 
Although toArray() will fix it, I think the fPlatformBPs.size() is not the right size.  It should probably be fPlatformBPs.get(ctx).size().  But instead, I usually use a size of 0 and let toArray() figure it out.

Comment 4 Francois Chouinard

2009-01-29 10:40:59 EST

Created attachment 124156 [details]
Updated fix

Good point. I committed the updated patch.

Comment 5 Pawel Piech

2009-01-29 12:02:40 EST

(In reply to comment #3)
>  But instead,
> I usually use a size of 0 and let toArray() figure it out.
Are you kidding me?  I didn't know you could do that!

Comment 6 Marc Khouzam

2009-01-29 12:08:21 EST

(In reply to comment #5)
> (In reply to comment #3)
> >  But instead,
> > I usually use a size of 0 and let toArray() figure it out.
> Are you kidding me?  I didn't know you could do that! 

:-)

From Collection.toArray(Object[] a):
"If the collection fits in the specified array, it is returned therein. Otherwise, a new array is allocated with the runtime type of the specified array and the size of this collection."

So I guess using 0 is not the most efficient, as it wastes the call to 
new myArray[0], but I figure it it worth the bugs one can avoid.