Community
Participate
Working Groups
Created attachment 191112 [details] Proposed patch Removing breakpoints in the UI is not properly handled in the TCFBreakpointsStatusListener. At the time the BreakpointListener is notified about breakpointRemoved(), the TCFBreakpointsModel has already removed the breakpoint from its mapping, therefore IBreakpoint bp = bp_model.getBreakpoint(id); always yields null. This causes exceptions being logged on disconnect. Steps to reproduce: - Launch a TCF session - Add a breakpoint (which gets planted) - Remove breakpoint - Disconnect Upon disconnect, decrementInstallCount() is called on the stale breakpoint which throws an exception: org.eclipse.debug.core.DebugException: Breakpoint does not have an associated marker. at org.eclipse.debug.core.model.Breakpoint.ensureMarker(Breakpoint.java:268) at org.eclipse.cdt.debug.internal.core.breakpoints.CBreakpoint.ensureMarker(CBreakpoint.java:295) at org.eclipse.debug.core.model.Breakpoint$2.run(Breakpoint.java:186) at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1975) at org.eclipse.debug.core.model.Breakpoint.setAttribute(Breakpoint.java:190) at org.eclipse.cdt.debug.internal.core.breakpoints.CBreakpoint.decrementInstallCount(CBreakpoint.java:274) at org.eclipse.tm.internal.tcf.cdt.ui.TCFBreakpointStatusListener$BreakpointListener$2.run(TCFBreakpointStatusListener.java:150)
Created attachment 191468 [details] Another way to fix it I suggest another way to fix it - patch attached. I think it is simpler and more reliable. I'm also a bit concerned about this code: IMarker marker = cbp.getMarker(); if (marker != null && marker.exists()) { cbp.decrementInstallCount(); } It is not synchronized with breakpoint deletion, so it can fail because of racing. May be we should change decrementInstallCount() and incrementInstallCount() to use jobs and scheduling rules same way as deleteTransientBreakpoint().
Created attachment 191490 [details] Eugene's fix + using jobs (In reply to comment #1) > I suggest another way to fix it - patch attached. > I think it is simpler and more reliable. Yes, I like it better too. > I'm also a bit concerned about this code: > > IMarker marker = cbp.getMarker(); > if (marker != null && marker.exists()) { > cbp.decrementInstallCount(); > } > > It is not synchronized with breakpoint deletion, so it can fail because of > racing. > May be we should change decrementInstallCount() and incrementInstallCount() to > use jobs and scheduling rules same way as deleteTransientBreakpoint(). I have added this check because I saw a lot of exceptions when deleting many breakpoints in a row from the Breakpoints view. There is no reliable way of synchronizing with breakpoint deletion (because of the default marker rule being null), so this check just reduces the chance of exception logging. However, I tried with workspace jobs and they seem to work fine. I am attaching another patch with these changes.
The code looks really good, but I'm still getting a lot of messages like: !ENTRY org.eclipse.debug.core 4 5012 2011-03-18 11:38:12.609 !MESSAGE Breakpoint does not have an associated marker. and !ENTRY org.eclipse.core.resources 4 376 2011-03-18 11:40:26.312 !MESSAGE Marker id 7746 not found. To reproduce I press and hold Ctrl+Shift+B in a source editor. Breakpoint deletion does not cause error every time, but it takes only a fraction of a second for the error dialog to pop up. And the messages would not go away if I change decrementInstallCount() like this: IMarker m = cbp.getMarker(); if (m == null || !m.exists()) return Status.OK_STATUS; cbp.decrementInstallCount(); ... I'm afraid this cannot be fixed for good without fixing the marker scheduling rule. BTW, I'm using 2 core machine, which might make the racings more severe.
Created attachment 191887 [details] Ignore CoreExceptions caused by expected racing (In reply to comment #3) > I'm afraid this cannot be fixed for good without fixing the marker scheduling > rule. Right, I think we can only ignore possible exceptions caused by the racing. This does not solve exceptions thrown elsewhere, of course. This patch also fixes a problem with toggling breakpoints after they have been disabled. The ICBreakpoint.SOURCE_HANDLE attribute must not change when updating breakpoint attributes, otherwise double clicking the breakpoint in the editor would not remove it.
I guess this is all we can do at this time. It works OK, exceptions are much less frequent now. I have committed the patch. Thanks!
