Repository navigation
"start" vs "continue" and the "ui-break-done" event #303
Description
I have a question about the use of the "ui-break-done" event. This seems to be configured to fire 50ms after any of the launch configurations have completed "loading". When this fires the "exec-run" command is executed. However, I noticed that the attach configuration which utilizes the "exec-continue" doesn't behave in this same way. The attach configurations wait to receive the "configurationDoneRequest" prior to executing the "exec-continue" command. I'm curious why the launch commands don't behave in the same way, waiting for the configuration to complete prior to executing the "exec-run" command.
It seems by using this arbitrary timeout to delay the "exec-run" command, it may result in not waiting for all of the configuration to have completed prior to starting execution. There are a number of open issues complaining about not hitting breakpoints set in the UI, and I wonder if this might potentially be the cause...if execution starts prior to the configuration completing.
I was recently experimenting with adding a breakpoint as part of the "configurationDoneRequest", but noticed that execution had started prior to my breakpoint command having been sent, which led me down this path to understand how the launch configurations trigger the execution of the "exec-run" command.
Is there a reason that this delay is used or should the correct behavior be similar to that for the continue which waits until the configuration has completed prior to beginning execution?
Activity
WebFreak001 commentedon Nov 23, 2021
that's some of the earliest code of this extension, the timeout was inserted probably to make sure, that the UI only calls commands on a valid initialized miDebugger instance. The event is just for the UI to switch into the "debugging is running" view, telling the user that they may now start issuing debug commands, it should not call any commands by itself. The debug host (vscode) may call debug methods on this event though.
50ms is just a nice length for a lot of work to be done, while looking to the user like it is almost instant (in UI design typically operations should not take longer than 100ms to be perceived as instant)
brownts commentedon Nov 23, 2021
Hi @WebFreak001, thanks for the follow-up.
I may be misunderstanding something, but it appears to me that the "ui-break-done" event is only used by the extension. From my investigation, it is emitted in all of the "launch" scenarios, but there appears to only be a single listener, as shown here, which is where the "exec-run" command is sent. That is why it appears to me that the "exec-run" is sent 50ms after "loading" completes, irregardless of whether the UI has finished initializing all of the breakpoints. In other words, there appears to be a race condition between when the "exec-run" is issued and when the breakpoint configuration completes.
Evidence of this seems to show up in at least issue #260. I looked through the logs provided in that issue and it appears that in cases where the breakpoints don't appear to be hit, the "exec-run" is issued prior to the breakpoint having been set (i.e., inferring "exec-run" from those logs based on the "Running executable" text in the output in relationship to "token:5" where the breakpoint is set). I noticed similar behavior in my own experimentation where "exec-run" was being issued too early, and that is what led me down this path. Just re-running the same scenario I could get instances where sometimes the breakpoint was set before the "exec-run" was issued, and sometimes afterwards, thus it seems pretty easy for this to happen.
Comparing the "launch" to the "attach", it appears the "attach" does not use this same behavior (i.e., timer) to kick off the "exec-continue", and in fact waits for the "configurationDoneRequest" to be sent prior to issuing that command, as shown here. So, if nothing else there appears to be an inconsistency between how "exec-run" is handled versus how "exec-continue" is handled. My guess is that the "exec-run" likely should not be issued until the "configurationDoneRequest" is received, similar to how the "exec-continue" is handled.
WebFreak001 commentedon Nov 23, 2021
oh given that FIXME comment I might have implemented that as a workaround because some stuff didn't work. But if it works now, feel free to adjust the initialization behavior to be more correct and not use timers.
brownts commentedon Nov 23, 2021
Ok, that makes sense. Yes, I had noticed that FIXME in the past and was a bit curious about it. I think I looked at one time and it had been added years ago. The "configurationDoneRequest" does seem to work fine for me, and is essential for continuing execution for the "attach" configurations, so I think it is safe to use for the "launch" configurations as well. I'll work on a PR to remove the timer approach and model it after how the "exec-continue" behaves.
jgueytat commentedon Nov 25, 2021
If so the #260 could be closed by the meantime. Well done @brownts ! Thanks guys
GitMensch commentedon Nov 25, 2021
@jgueytat Did you already verified if #260 is actually fixed after applying the changes in #304?
GitMensch commentedon Nov 26, 2021
fixed by #304
jgueytat commentedon Nov 29, 2021
Hi guys the #260 seems to work properly now with the 25.1 but I am enable to test it on Gitpod which was the best place to see the bug.
jgueytat commentedon Nov 29, 2021
For the #260 on Gitpod with the version v0.25.1 I still get the warning and no breakpoint.
warning: Error disabling address space randomization: Operation not permittedjgueytat commentedon Nov 29, 2021
With VSCode and WSL and the exact same configuration. It seems I get the breakpoint properly.
brownts commentedon Nov 29, 2021
Hi @jgueytat, I'm a little confused. Did you test this with the latest from this repository or with v0.25.1? The reason I ask is because, from the comments above, it appears you were testing with the previously released version and this fix would not have been incorporated into that version. This fix will be in an upcoming release which has not yet occurred. Is it possible for you to test with the latest in this repository?
jgueytat commentedon Nov 30, 2021
I checked again with the v0.25.1 not master.
I don't know yet how to compile and test a plugin. That beeing said I would find really cool to develop the plugin in Gitpod itself. ;) I'll give that a try.