False resume events fired while adding breakpoints to a running debugtarget

[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.
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

### Eclipse Workspace Patch 1.0
#P org.eclipse.cdt.debug.mi.core
Index: cdi/org/eclipse/cdt/debug/mi/core/cdi/BreakpointManager.java
===================================================================
RCS file: /cvsroot/tools/org.eclipse.cdt/all/org.eclipse.cdt.debug.mi.core/cdi/org/eclipse/cdt/debug/mi/core/cdi/BreakpointManager.java,v
retrieving revision 1.54
diff -u -r1.54 BreakpointManager.java
--- cdi/org/eclipse/cdt/debug/mi/core/cdi/BreakpointManager.java 2 Jun 2009 19:39:38 -0000 1.54
+++ cdi/org/eclipse/cdt/debug/mi/core/cdi/BreakpointManager.java 8 Jul 2009 06:10:41 -0000
@@ -162,10 +162,23 @@
  }
 
  void resumeInferior(Target target, boolean shouldRestart) throws CDIException {
- ((EventManager) getSession().getEventManager()).allowProcessingEvents(true);
  if (shouldRestart) {
  target.resume();
  }
+ /* enable event processing after sending the resume event.
+ * this is to prevent false resume events.
+ * The sleep(...) prevents a race condition where event processing gets enabled
+ * before the response to the resume command is received.
+ */
+ try
+ {
+ Thread.sleep(200);
+ }
+ catch (InterruptedException ie)
+ {
+ //ignore
+ }
+ ((EventManager) getSession().getEventManager()).allowProcessingEvents(true);
  }
 
  public void deleteBreakpoint(MISession miSession, int no) {

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

[Elena Laskavaia]

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.

Abeer Bagul wrote:

> 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
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> cdt-debug-dev mailing list
> cdt-debug-dev@...
> https://dev.eclipse.org/mailman/listinfo/cdt-debug-dev

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