Community
Participate
Working Groups
We would like JDT to provide us with an extension point that allows us to contribute actions to be run whenever a Java breakpoint is hit. The extension point allows clients to contribute an "action delegate". The action delegate is called whenever the java breakpoint is hit. The action delegates will run whatever action is required. The delegates will have an option to vote if the target should be suspended or resumed. The action delegate will be called after the condition on the breakpoints are evaluated, but before the java breakpoint listener are called. Clients may contribute a list of actions to the breakpoint by setting an attribute on the breakpoint marker. The new breakpoint attribute consists of a comma-delimited list of action delegates ids. Whenever the breakpoint is hit, JDT goes through the listed action delegates and notify them to run actions sequentially.
Created attachment 122541 [details] Patch to org.eclipse.jdt.debug & org.eclipse.jdt.debug.ui Patch to implement extension point. Things not sure about: 1) Is it a good idea to set the thread to running again if StepHandler has won a suspend vote? 2) Should JDIThread handle the action delegates, or should the individual breakpoints handle invoking the action delegates. 3) I currently setSuspendQuiet to true when the action delegates are being handled. Not sure if this would introduce bad side-effects. 4) I also temporarily unlock JDIThread with the action delegates are handled. Not sure if this will cause bad side effects as other threads can get unlocked unexpectedly. To-Do: 1) Need to handle suspend vote conflicts when the user has manually suspended the target while the action delegates report to resume. If the user manually suspends the target, the "suspend action" is not involved in the suspend votes by the EventDispatcher. The target will get suspended, but when the action delegates reply to resume the target, the target will get resumed. The expected behavior is that the target should suspend if the user has explicitly suspended the session. 2) There seems to be timing problems where the Debug view sometimes report wrong states while evaluations are done by action delegates. The thread will be reported as suspended when it should be in "evaluation" or "running" states. 3) Need more testing 4) Need better docs
Created attachment 122719 [details] updated patch * Addresses the problem when stepping runs away if the action delegates votes to resume on step-end. * Fixed the problem when the Debug view shows wrong states while evaluations are done by action delegates. - icons / hasVariables do not handle suspend quiet properly. * Fixed NPE in JDIVoidValue if evaluations are done after the target is terminated. * Tried to handle the case when the target is interrupted when action delegates are notified. This is when the action delegates may vote to resume, while the user has manually suspend. We need to detect this case, and honor the suspend vote by the suspend action. This case is still not working properly 100% of the time. The debug view sometimes end up in a bad states where the label update job is stuck waiting for evaluations to complete, and result in empty labels in the debug view. But there is not more evaluation in the queue to wake up the label update job. (not sure how to fix this yet.) * Added docs + copyright Darin, I think this patch is ready for review and feedback. I will continue to look at the last problem and see what I can find. Thanks...
Created attachment 125282 [details] alternate implementation This patch is similar to Sam's proposal, but uses existing IJavaBreakpointListeners rather than introducing a new interface. It has minimal testing as of yet (only one automated test). However, it does validate that an expression can be run during a callback. I find the breakpoint listener extensions are hard to configure, since the framework instantiates the listener that will be called. This means the listener has to get its configuration/state from somewhere else (perhaps other breakpoint attributes?). Sam, will this be an issue?
(More thoughts...) This patch removes the problem of having the IJavaThread locked while notifying listeners, and allows evaluations to be performed. With these improvements, would it be sufficient to just use standard IJavaBreakpoinListeners without the extension point? (i.e. programmatically register the listeners). The extension point supports breakpoint specific listeners, where as the programmatically registered listeners recieve notification for all breakpoints. If standard listeners are sufficient, would you need support for breakpoint specific listeners?
Hi Darin, Here are a couple of things I found after testing out your patch: 1. Action votes to resume, when we are at step end - The debug model gets into a weird state. The user can no longer suspend/resume the session. I added code in StepHandler and that seems to have fixed the problem: public void wonSuspendVote(Event event, JDIDebugTarget target) { setSuspendedQuiet(false); setRunning(false); } 2. The breakpoint gets hit when we are in the middle of a step over. The listener cannot run the evaluation successfully. JDIThread#canRunEvaluation returns false, because the thread is in stepping mode. It is blocking the evaluations. 3. I have a breakpoint that performs evaluations on breakpoint hit, then resume. This breakpoint is on a loop and gets hit many times. If I hit suspend when these evaluations are being peformed, suspend fails. Sometimes the suspend action is disabled for a long period of time, and I cannot suspend. When it manages to suspend, the Debug view continues to flicker with action enablement updates... and the session seems to be in a bad state. Thanks...
(In reply to comment #5) > Here are a couple of things I found after testing out your patch: > 1. Action votes to resume, when we are at step end - The debug model gets into > a weird state. The user can no longer suspend/resume the session. I added > code in StepHandler and that seems to have fixed the problem: > public void wonSuspendVote(Event event, JDIDebugTarget target) { > setSuspendedQuiet(false); > setRunning(false); > } This looks like a bug that existed with the breakpoint listeners forever. The problem is that the thread updates its state to running/suspended before the vote is complete. I'm looking at other ways to fix this.
Created attachment 125446 [details] updated patch This addresses problems (1) and (2). Problem (3) also looks like an existing problem. Suspending a thread does not suspend/clear pending evaluations or handle voters that effectively resume a thread.
Thanks a lot Darin, This patch works a lot better. I can get all the different stepping scenarios to work. I will test out different scenarios more. I still notice a few other things when trying out this patch: Scenario 1: 1. Suspend a a breakpoint 2. I have a few breakpoints after, and will run evaluations when breakpoints get it... These breakpoints will vote resume 3. Stackframe is selected and I hit resume ==> the thread shows that it has hit a breakpoint, but the icon shows up as running. Debug actions show up as if the the thread is in suspend states ==> after a while, the thread shows up as evaluating... and actions get updated again ==> I think this is related to the fact that we need quiet evaluations. Or, that the suspend event from the breakpoint is overriding the resume event, causing wrong labels to be displayed ===> In some cases, I get the thread expanded, and we ended up with empty labels in the Debug view. Scenario 2: 1. I have a bunch of breakpoints that run evals, and vote resume. The evals are being run. 2. Terminate the session ===> I received a dialog saying that the evaluations have errors, and we receive an IO exception. ===> Perhaps we can supress this error?
Created attachment 125677 [details] updated patch This update addresses problems 1,2 and 3 from comment#5. When the user suspends, any existing evaluation is terminated (if possible) at the end of the next instruction, we wait for any method invocation to complete, and breakpoints listener notification is aborted (i.e. if there are 5 listeners and we are notifying listener 3 of a breakpoint, listeners 4 and 5 will not be notified). The user suspend overrides breakpoints. Re: comment#8 Scenario 1 - I think this is due to the imiplicit evaluation events being fired and the view updating asycnhronously with the model. This would require QUIET evaluations. Scenario 2 - we can likely suppress the error when a target terminates and an evaluation is in progress.
An issue that is not yet accounted for: * a thread hits a breakpoint * notifies breakpoint listeners * one listener starts and blocks on a long-running operation (*not* an eval) We currently don't wait for the listener to complete, unless it happens to be doing an evaluation (since we account for evaluations explicitly). The thread will fire a suspend event even though the listener is still running. I think we should we wait for the listener to complete (based on a timeout threshold), and throw an exception if the listener does not complete within the timeout (similar to waiting on evaluations to terminate/complete).
(In reply to comment #10) > I think we should we wait for the listener to complete (based on a timeout > threshold), and throw an exception if the listener does not complete within the > timeout (similar to waiting on evaluations to terminate/complete). > I agree, I think we should wait and throw exception if the action is long running. I have opened Bug 265229 to request for silent evaluations. I have tried out your latest patch, and confirmed that all scenarios from Comment #5 are resolved. It was difficult to try to suspend the debug session, because the evaluations cause the suspend action to be disabled most of the time.
Created attachment 125969 [details] update This patch (still work in progress) is updated to handle a running evaluation when the VM disconnects/terminates. As well, it is updated to support quiet evaluations. I would propose that quiet evaluations only be allowed for breakpoint listeners. The current patch fires MODEL_SPECIFIC events instead of standard evaluation events. However, we may not need to fire any events. The content providers and model proxies for the Java debugger only reveal frames/suspension after breakpoint listeners have been notified (but the model is still programatically suspended/can retrieve frames). The patch is updated to handle manual suspend requests of the target/and or thread.
Created attachment 126237 [details] patch This patch is cleaned up. The extension point allows for listeners to be contributed with a filter. This allows listeners to be registered for all breakpoints vs. specific breakpoints. In the future, we could allow for more options (like all method breakpoints, or all breakpoints in a specific type, etc).
Created attachment 126343 [details] and once more... This patch changes the implementation of conditional breakpoints. The condition is now an atomic part of the breakpoint, rather than a general purpose listener. This means that "hit" notifications are only sent to listeners if a condition evaluates to true. I think this is a better abstraction/model. One could imagine conditions being executed on the back end at some point, which would result in the same pattern.
Committed to HEAD. Added a couple more tests and updated documentation to point to new extension point.
Please verify, Samantha. Please open new bugs if required.
