False resume events fired while adding breakpoints to a running debugtarget

Discussion:

Abeer Bagul

2009-07-08 06:15:06 UTC

Hi All,

Currently, the way a breakpoint is planted while a debugtarget is running,
leads to false resume events.
When a user tries to add breakpoints to a running target, the
BreakpointManager suspends the target, plants the breakpoints and then
resumes the target.
Look at:
http://bugs.eclipse.org/bugs/show_bug.cgi?id=166660#c23

Before suspending the target, the code disables event processing so that
false suspend events are not generated. But the code enables event
processing before sending the resume command, leading to false resume
events.

We have a debugeventlistener which is behaving wrongly due to the false
resume events.

Can the code in BreakpointManager.resumeInferior() be changed so that event
processing is enabled after the resume command is sent? Since event
processing and resuming happen on different threads, we had to insert a
Thread.sleep() call before enabling the event processing (to prevent a race
condition in which the event processing is enabled before the response to
the resume command is recieved).

I have attached a patch which prevents these false resume events. Should I
attach this patch to bug 166660?

Thanks
Abeer

Elena Laskavaia

2009-07-08 12:44:06 UTC

Permalink

1) You always attach patches to prs.
2) In this particular case I don't think it would be accepted. If you enable events after target is resumed it can miss the event which is even worse.
Using sleep to avoid race condition is not a solution either.

Post by Abeer Bagul
Hi All,
Currently, the way a breakpoint is planted while a debugtarget is
running, leads to false resume events.
When a user tries to add breakpoints to a running target, the
BreakpointManager suspends the target, plants the breakpoints and then
resumes the target.
http://bugs.eclipse.org/bugs/show_bug.cgi?id=166660#c23
Before suspending the target, the code disables event processing so that
false suspend events are not generated. But the code enables event
processing before sending the resume command, leading to false resume
events.
We have a debugeventlistener which is behaving wrongly due to the false
resume events.
Can the code in BreakpointManager.resumeInferior() be changed so that
event processing is enabled after the resume command is sent? Since
event processing and resuming happen on different threads, we had to
insert a Thread.sleep() call before enabling the event processing (to
prevent a race condition in which the event processing is enabled before
the response to the resume command is recieved).
I have attached a patch which prevents these false resume events. Should
I attach this patch to bug 166660?
Thanks
Abeer
------------------------------------------------------------------------
_______________________________________________
cdt-debug-dev mailing list
https://dev.eclipse.org/mailman/listinfo/cdt-debug-dev

Abeer Bagul

2009-07-09 04:27:37 UTC

Permalink

Hi Elena,

The issue highlighted by you is that if after resume, immediately a
breakpoint is hit, the suspend event will get lost because event processing
was turned off. So the correct code to write is:
1. enabled event processing
2. resume the debugtarget

Reading the call hierarchy of the methods isAllowingProcessingEvents() and
allowProcessingEvents(boolean allowed), it seems these methods are called to
suppress only suspend and resume events.

The pattern is:
1. Developer wants to suspend gdb in the background
2. The suspend event should be ignored.
3. So disable event processing before the suspend call.
4. After suspend command is sent, enable event processing.

again,
1. Developer wants to resume gdb in the background
2. This specific resume event should be ignored.
3. After this specific resume event is ignored, if there is any other resume
event, process it.

So isAllowingProcessingEvents() and allowProcessingEvents(boolean allowed)
is (currently) used for the above two patterns only.

Can we add make the event processing enable / disable more granular to make
two patterns more clear?
Add the following four methods (two for suspend and two for resume):

//1.
private int ignoreSuspendCount = 0;
public void setIgnoreSuspendEvent(int ignoreCount)
{
this.ignoreSuspendCount = ignoreCount;
}

public boolean ignoreSuspendEvent()
{
boolean ignore = (ignoreSuspendCount > 0);
if (ignore)
ignoreSuspendCount--;
return ignore;
}

//2.
private int ignoreResumeCount = 0;
public void setIgnoreResumeEvent(int ignoreCount)
{
this.ignoreResumeCount = ignoreCount;
}

public boolean ignoreResumeEvent()
{
boolean ignore = (ignoreResumeCount > 0);
if (ignore)
ignoreResumeCount--;
return ignore;
}

Rather than calling allowProcessingEvents(boolean allowed), the user can
call:
setIgnoreSuspendEvent(1) before suspending gdb
setIgnoreResumeEvent(1) before resuming gdb. After resuming gdb, if the
breakpoint is hit, the next suspend event will not be missed.

This also has the good side effect that false resume events are avoided.

Sorry if this reply is going in the wrong direction.

Thanks
Abeer Bagul
Software Engineer, Tensilica India

---------------------------------------------------------------------
Message: 4
Date: Wed, 08 Jul 2009 08:44:06 -0400
From: Elena Laskavaia <elaskavaia-***@public.gmane.org>
Subject: Re: [cdt-debug-dev] False resume events fired while adding
breakpoints to a running debugtarget
To: CDT Debug developers list <cdt-debug-dev-j9T/***@public.gmane.org>
Message-ID: <4A549496.7040004-***@public.gmane.org>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed

1) You always attach patches to prs.
2) In this particular case I don't think it would be accepted. If you enable
events after target is resumed it can miss the event which is even worse.
Using sleep to avoid race condition is not a solution either.

Elena Laskavaia

2009-07-09 14:38:57 UTC

Permalink

That is sounds reasonable. You want to send patch/pr for that?

Post by Abeer Bagul
Hi Elena,
The issue highlighted by you is that if after resume, immediately a
breakpoint is hit, the suspend event will get lost because event
1. enabled event processing
2. resume the debugtarget
Reading the call hierarchy of the methods isAllowingProcessingEvents()
and allowProcessingEvents(boolean allowed), it seems these methods are
called to suppress only suspend and resume events.
1. Developer wants to suspend gdb in the background
2. The suspend event should be ignored.
3. So disable event processing before the suspend call.
4. After suspend command is sent, enable event processing.
again,
1. Developer wants to resume gdb in the background
2. This specific resume event should be ignored.
3. After this specific resume event is ignored, if there is any other
resume event, process it.
So isAllowingProcessingEvents() and allowProcessingEvents(boolean
allowed) is (currently) used for the above two patterns only.
Can we add make the event processing enable / disable more granular to
make two patterns more clear?
//1.
private int ignoreSuspendCount = 0;
public void setIgnoreSuspendEvent(int ignoreCount)
{
this.ignoreSuspendCount = ignoreCount;
}
public boolean ignoreSuspendEvent()
{
boolean ignore = (ignoreSuspendCount > 0);
if (ignore)
ignoreSuspendCount--;
return ignore;
}
//2.
private int ignoreResumeCount = 0;
public void setIgnoreResumeEvent(int ignoreCount)
{
this.ignoreResumeCount = ignoreCount;
}
public boolean ignoreResumeEvent()
{
boolean ignore = (ignoreResumeCount > 0);
if (ignore)
ignoreResumeCount--;
return ignore;
}
Rather than calling allowProcessingEvents(boolean allowed), the user can
setIgnoreSuspendEvent(1) before suspending gdb
setIgnoreResumeEvent(1) before resuming gdb. After resuming gdb, if the
breakpoint is hit, the next suspend event will not be missed.
This also has the good side effect that false resume events are avoided.
Sorry if this reply is going in the wrong direction.
Thanks
Abeer Bagul
Software Engineer, Tensilica India
---------------------------------------------------------------------
Message: 4
Date: Wed, 08 Jul 2009 08:44:06 -0400
Subject: Re: [cdt-debug-dev] False resume events fired while adding
breakpoints to a running debugtarget
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
1) You always attach patches to prs.
2) In this particular case I don't think it would be accepted. If you
enable events after target is resumed it can miss the event which is
even worse.
Using sleep to avoid race condition is not a solution either.

resumeInferior() be changed so that

Post by Abeer Bagul
event processing is enabled after the resume command is sent? Since
event processing and resuming happen on different threads, we had to
insert a Thread.sleep() call before enabling the event processing (to
prevent a race condition in which the event processing is enabled before
the response to the resume command is recieved).
I have attached a patch which prevents these false resume events. Should
I attach this patch to bug 166660?
Thanks
Abeer
------------------------------------------------------------------------
_______________________________________________
cdt-debug-dev mailing list
https://dev.eclipse.org/mailman/listinfo/cdt-debug-dev

------------------------------------------------------------------------
_______________________________________________
cdt-debug-dev mailing list
https://dev.eclipse.org/mailman/listinfo/cdt-debug-dev