fix: do not send continue events #307
Conversation
…f the normal flow
Codecov Report
@@ Coverage Diff @@
## master #307 +/- ##
==========================================
- Coverage 70.17% 70.05% -0.12%
==========================================
Files 5 5
Lines 1016 1012 -4
Branches 161 161
==========================================
- Hits 713 709 -4
Misses 303 303
Continue to review full report at Codecov.
|
| @@ -279,10 +279,6 @@ class PhpDebugSession extends vscode.DebugSession { | |||
| }) | |||
| connection.on('error', disposeConnection) | |||
| connection.on('close', disposeConnection) | |||
| connection.on('before-execute-command', () => { | |||
There was a problem hiding this comment.
Is this event then still used at all?
There was a problem hiding this comment.
@felixfbecker No, this PR completely removes the event, it turns out that it causes problems to send the continued event. In several scenarios it duplicates other events sent that also tell vscode that the code is running. The second notice sometimes gets sent after it is paused again, and ends up making vsc think it continued when it is actually still paused.
| @@ -679,11 +679,6 @@ export class Connection extends DbgpConnection { | |||
| const data = iconv.encode(commandString, ENCODING) | |||
| this._pendingCommands.set(transactionId, command) | |||
| this._pendingExecuteCommand = command.isExecuteCommand | |||
| if (this._pendingExecuteCommand) { | |||
There was a problem hiding this comment.
@felixfbecker The flag itself is still used to prevent that deadlock where it waits for breakpoint init call on a connection that is hung running code.
There was a problem hiding this comment.
In #301 you said
I think the "opposite" problem is happening now which is worse than the original problem that it fixes, now instead of looking like it is paused when it is not, it looks like the code continued when it did not. So like I said above I'll try to come up with a solution that will allow neither to happen, but if I do not make progress today I'll put that PR in place since the old problem is not as bad.
Could you explain in what case before-execute-command gets fired when the program is not actually continuing?
|
@felixfbecker It helps to see the description for that event: /** Event message for 'continued' event type.
The event indicates that the execution of the debuggee has continued.
Please note: a debug adapter is not expected to send this event in response to a request that implies that execution continues, e.g. 'launch' or 'continue'.
It is only necessary to send a 'continued' event if there was no previous request that implied this.
*/So in other words, if there is somewhere that we are making the code continue that is not part of a continue request from vscode, that is when to use it. One thing that comes to mind is if the code has paused somewhere that we ignore and we want to continue automatically, we might want to then notify vscode that it has continued. My original statement was before I fully looked into it. It would be more accurate to say that we send the continued event in addition to the normal response to vscode, and sometimes that can cause vscode to think the code is running when it is actually paused. One example of when this could happen: it sends as a result of initializing xdebug as part of the first run command. It sends that continued event. That event is sent async. It could actually get to a breakpoint and be paused on the breakpoint at the time vscode receives the event, so vscode incorrectly thinks the code has continued when it has not. That is what was happening on #301 I hope that makes sense? |
|
Ah, that makes sense to me. |
felixfbecker
changed the title
|
🎉 This PR is included in version 1.12.6 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
A debug adapter is not expected to send this event in response to a request that implies that execution continues, e.g. 'launch' or 'continue'. It is only necessary to send a 'continued' event if there was no previous request that implied this. Note: there is still the problem that it will seem to "hang" when you continue and the PHP code takes a while to run, xdebug#308 will fix that. Fixes xdebug#301
…f the normal flow
This fixes #301
Note: there is still the problem that it will seem to "hang" when you continue and the PHP code takes a while to run, #308 will fix that.