Bug 269182 - [breakpoints] Breakpoint does not have an associated marker error in the .log file
Summary: [breakpoints] Breakpoint does not have an associated marker error in the .log...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.5 RC2   Edit
Assignee: JDT-Debug-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 275876 276021 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-03-18 09:15 EDT by Olivier Thomann CLA
Modified: 2009-06-22 16:59 EDT (History)
8 users (show)

See Also:
Michael_Rennie: review+
chanskw: review+


Attachments
one proposed fix (908 bytes, patch)
2009-05-14 11:30 EDT, Michael Rennie CLA
no flags Details | Diff
patch (13.57 KB, patch)
2009-05-14 13:53 EDT, Darin Wright CLA
no flags Details | Diff
patch (14.05 KB, patch)
2009-05-14 14:04 EDT, Darin Wright CLA
no flags Details | Diff
patch (14.14 KB, patch)
2009-05-14 14:45 EDT, Darin Wright CLA
no flags Details | Diff
updated patch (88.33 KB, patch)
2009-05-15 11:53 EDT, Darin Wright CLA
no flags Details | Diff
update (51.99 KB, patch)
2009-05-15 12:09 EDT, Darin Wright CLA
no flags Details | Diff
all tests (21.49 KB, patch)
2009-05-15 12:13 EDT, Darin Wright CLA
no flags Details | Diff
patch (20.08 KB, patch)
2009-05-15 14:13 EDT, Darin Wright CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Thomann CLA 2009-03-18 09:15:39 EDT
Using eclipse.buildId=I20090313-0100
java.version=1.6.0_10
java.vendor=Sun Microsystems Inc.
BootLoader constants: OS=win32, ARCH=x86, WS=win32, NL=fr_CA
Command-line arguments:  -os win32 -ws win32 -arch x86 -debug -consolelog -console:

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.jdt.internal.debug.core.breakpoints.JavaBreakpoint.getBreakpointListeners(JavaBreakpoint.java:1252)
at org.eclipse.jdt.internal.debug.core.JDIDebugPlugin$AbstractNotifier.notifyListeners(JDIDebugPlugin.java:456)
at org.eclipse.jdt.internal.debug.core.JDIDebugPlugin$BreakpointNotifier.notify(JDIDebugPlugin.java:559)
at org.eclipse.jdt.internal.debug.core.JDIDebugPlugin.fireBreakpointRemoved(JDIDebugPlugin.java:395)
at org.eclipse.jdt.internal.debug.core.breakpoints.JavaBreakpoint.fireRemoved(JavaBreakpoint.java:917)
at org.eclipse.jdt.internal.debug.core.breakpoints.JavaBreakpoint.removeFromTarget(JavaBreakpoint.java:659)
at org.eclipse.jdt.internal.debug.core.breakpoints.JavaLineBreakpoint.removeFromTarget(JavaLineBreakpoint.java:171)
at org.eclipse.jdt.internal.debug.core.model.JDIDebugTarget.breakpointRemoved(JDIDebugTarget.java:1226)
at org.eclipse.debug.internal.core.BreakpointManager$BreakpointNotifier.run(BreakpointManager.java:928)
at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
at org.eclipse.debug.internal.core.BreakpointManager$BreakpointNotifier.notify(BreakpointManager.java:951)
at org.eclipse.debug.internal.core.BreakpointManager.fireUpdate(BreakpointManager.java:865)
at org.eclipse.debug.internal.core.BreakpointManager.removeBreakpoints(BreakpointManager.java:476)
at org.eclipse.debug.internal.core.BreakpointManager$BreakpointManagerVisitor.update(BreakpointManager.java:718)
at org.eclipse.debug.internal.core.BreakpointManager.resourceChanged(BreakpointManager.java:660)
at org.eclipse.core.internal.events.NotificationManager$2.run(NotificationManager.java:290)
at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37)
at org.eclipse.core.internal.events.NotificationManager.notify(NotificationManager.java:284)
at org.eclipse.core.internal.events.NotificationManager.broadcastChanges(NotificationManager.java:148)
at org.eclipse.core.internal.resources.Workspace.broadcastBuildEvent(Workspace.java:297)
at org.eclipse.core.internal.events.AutoBuildJob.doBuild(AutoBuildJob.java:143)
at org.eclipse.core.internal.events.AutoBuildJob.run(AutoBuildJob.java:238)
at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)

I get this quite often when I have a breakpoint inside a class. I make changes after the breakpoint is hit inside the debugger and on the rebuild I get it sometimes. Might be related to HCR.
Comment 1 Markus Keller CLA 2009-04-15 13:49:57 EDT
To reproduce in I20090414-0800:
- set a breakpoint
- close the enclosing project
Comment 2 Michael Rennie CLA 2009-05-14 10:01:19 EDT
*** Bug 276021 has been marked as a duplicate of this bug. ***
Comment 3 Dani Megert CLA 2009-05-14 10:21:20 EDT
Any idea why I might see it more frequently? Any chance for a fix?
Comment 4 Darin Wright CLA 2009-05-14 10:55:32 EDT
This is happenning more fequently due the enhanced breakpoint listener support we added (bug 260910). When a breakpoint is removed, we are trying to notify its "breakpoint specific" listeners, which were stored in the breakpoint. Of course, this will not work since the underlying marker has been deleted. Should address for RC1 or RC2.
Comment 5 Darin Wright CLA 2009-05-14 11:00:27 EDT
Sam, there is an issue when notifying "breakpoint specific" listeners of breakpoint removal - we no longer know who the breakpoint specific listeners are since the underlying marker is gone. For 3.5, is it OK to not notify the breakpoint specific listeners (only the global ones?).
Comment 6 Michael Rennie CLA 2009-05-14 11:30:47 EDT
Created attachment 135798 [details]
one proposed fix

One way we could solve this is to not log the exception - since the breakpoint listeners will not be notified either way.
Comment 7 Samantha Chan CLA 2009-05-14 12:03:18 EDT
For 3.5, I think we are ok to not notify the breakpoint-specific listener on remove.  I think our model may be able to get around this by registering a global listener, and let our own global listener notify the breakpoint-specific listener when a breakpoint removed event is received.

Is it your long-term solution though?  I think the breakpoint specific listeners still need to be notified.  The listener could use the breakpoint removed notification to do clean up.  Is it possible to get notified before the breakpoint marker is removed?  

Do we need to document that the breakpoint-specific listener will not receive a breakpoint removed event?
Comment 8 Darin Wright CLA 2009-05-14 12:26:34 EDT
I think we should do the right thing for 3.5, rather than doc the wrong behavior. I have a patch, just needs a few more tests.
Comment 9 Dani Megert CLA 2009-05-14 13:37:42 EDT
>I think we should do the right thing for 3.5, 
You mean to find out why we get this exception and fix the real cause?
Comment 10 Darin Wright CLA 2009-05-14 13:49:08 EDT
(In reply to comment #9)
> >I think we should do the right thing for 3.5, 
> You mean to find out why we get this exception and fix the real cause?

No - we know the real cause. I mean provide the real fix, so listeners can be notified after a breakpoint marker is deleted.

Comment 11 Dani Megert CLA 2009-05-14 13:51:40 EDT
>No - we know the real cause.
And fixing that is out of reach for 3.5  guess. Is there a bug number?
Comment 12 Darin Wright CLA 2009-05-14 13:53:42 EDT
Created attachment 135830 [details]
patch

Caches breakpoint listeners in the breakpoint model object so they can be retrieved after the underlying marker is deleted. Also fixes the problem of adding duplicate breakpoint listeners to a breakpoint (i.e. same identifier can only be added once).
Comment 13 Darin Wright CLA 2009-05-14 13:55:00 EDT
(In reply to comment #11)
> >No - we know the real cause.
> And fixing that is out of reach for 3.5  guess. Is there a bug number?

It's within reach. The patch is the suggested fix.
Comment 14 Darin Wright CLA 2009-05-14 14:04:01 EDT
Created attachment 135831 [details]
patch

Fixes timing problem in the test.
Comment 15 Darin Wright CLA 2009-05-14 14:45:41 EDT
Created attachment 135843 [details]
patch

Fix to JUnit test. The test is harder to write than the code since breakpoint/resource change notification can happen in the same or in a different thread.
Comment 16 Darin Wright CLA 2009-05-14 15:15:25 EDT
Pushing to RC2. The test is flaky, so we need more time to investigate.
Comment 17 Samantha Chan CLA 2009-05-14 17:02:26 EDT
Reviewed the code, the code looks good.

The test is also flaky for me.  I tried removing the timeout on the wait for the remove notification.  If there is no timeout, the test never finished.  It looks like that the wait has blocked the breakpoint removed event being sent to the listener.  I also notice that the test is done on the main thread, while the notification of the breakpoint removed event is also done on the main thread.  Wondering if the main thread is being held up, causing us not be able to send notifications.
Comment 18 Darin Wright CLA 2009-05-14 17:09:39 EDT
Thanks Sam. We'll do some more review and ensure that the failure is just a problem with the test. The exception logging itself is not harmful other than distracting for users.
Comment 19 Darin Wright CLA 2009-05-14 22:55:36 EDT
I found that when the test fails, the breakpoint manager is not receiving a resource delta - it's like we're missing the delta for the deleted marker. When I force a build afte deleting the marker in a workspace runnable, a resource delta is sent, but it has no marker deltas. So it's a mystery as to where the marker deltas are going.
Comment 20 Darin Wright CLA 2009-05-15 10:47:30 EDT
The inconsistent test result is due to the fact that the breakpoint manager is listening to POST_BUILD resource deltas to process removed markers. Sometimes the POST_BUILD delta does not contain the marker deletion - it was sent in the POST_CHANGE delta, but not the POST_BUILD. Sometimes, it appears in both. I need to find out why this is.

This is really a different/second bug - i.e. the breakpoint manager is not properly deleting breakpoints (and notifying listeners) when markers are deleted.
Comment 21 Michael Rennie CLA 2009-05-15 11:22:47 EDT
(In reply to comment #20)
> This is really a different/second bug - i.e. the breakpoint manager is not
> properly deleting breakpoints (and notifying listeners) when markers are
> deleted.
> 

It would interesting to remember why we decided to listen to POST_BUILD deltas for this case, since the updating of breakpoints does not require a built state, but does care when the resource they are set on is changed. We should examine the impact of changing to listening to POST_CHANGE deltas as opposed to POST_BUILD.
Comment 22 Darin Wright CLA 2009-05-15 11:53:38 EDT
Created attachment 136021 [details]
updated patch

This patch changes the breakpoint manager to be a POST_CHANGE listener. All tests pass reliably with this change. It actually simplifies code in the breakpoint manager where we had two listeners (one POST_CHANGE, one POST_BUILD to reconcile changes between CHANGE/BUILD notification).
Comment 23 Michael Rennie CLA 2009-05-15 12:08:56 EDT
(In reply to comment #22)
> All
> tests pass reliably with this change. 

Most of the tests are commented out in this patch :)

Comment 24 Darin Wright CLA 2009-05-15 12:09:23 EDT
Created attachment 136024 [details]
update

incorrectly commented out some tests in previous patch.
Comment 25 Darin Wright CLA 2009-05-15 12:13:24 EDT
Created attachment 136026 [details]
all tests

All tests are back in this one. Ug.
Comment 26 Michael Rennie CLA 2009-05-15 12:41:26 EDT
> All tests are back in this one. Ug.
> 

Code looks good, all tests pass on Linux and Windows.
Comment 27 Darin Wright CLA 2009-05-15 13:43:51 EDT
Talking with John, we should theoretically get the same marker deltas reported in the POST_BUILD as the POST_CHANGE. The difference is that the workspace is open for changes during the POST_BUILD and not during the POST_CHANGE. This could be important for breakpoint listeners (if they have to mofidy a breakpoint marker in response to a breakpoint add/change). In the case of remove, they obviously can't modify the marker.
Comment 28 Darin Wright CLA 2009-05-15 14:13:17 EDT
Created attachment 136040 [details]
patch

John wins... he pointed out a timing issue with the test. If the breakpoint is created/deleted before the resource delta is fired, the POST_BUILD notification has no deletion delta since create + delete = no changes (sum of changes). If we force a build after breakpoint creation, everything works as it should.

I vote to leave the breakpoint manager as a post build listener, since it is notifying breakpoint listeners of changes that may need to modify the workspace in a callback.

I have modified the debug tests to force a build after each breakpoint creation to ensure "add" deltas are sent. Removed changes from breakpoint manager.
Comment 29 Samantha Chan CLA 2009-05-19 10:38:05 EDT
+1.  Test passes for me too.
Comment 30 Darin Wright CLA 2009-05-19 12:09:34 EDT
Released for RC2.
Comment 31 Darin Wright CLA 2009-06-15 11:29:35 EDT
*** Bug 275876 has been marked as a duplicate of this bug. ***
Comment 32 Curtis Windatt CLA 2009-06-22 16:59:42 EDT
Verified by multiple committers.