Debugger: add dedicated message for VSCode watch#1542
Debugger: add dedicated message for VSCode watch#1542matetokodi wants to merge 1 commit intoSamsung:masterfrom
watch#1542Conversation
matetokodi
requested review from
bbrto21,
clover2123,
ksh8281 and
zherczeg
as code owners
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| sendType(ESCARGOT_MESSAGE_PARSE_DONE); | ||
|
|
||
| if (enabled() && pendingWait()) { | ||
| if (enabled() && pendingWait() && !m_watchEval) { |
There was a problem hiding this comment.
The condition uses !m_watchEval without any prior check that m_watchEval has been initialized. If m_watchEval is an uninitialized member, reading it causes undefined behavior and can lead to unpredictable skipping of the breakpoint wait. Ensure m_watchEval is initialized (e.g., in the constructor) before this check or guard the access with a safe default value.
src/debugger/Debugger.cpp
Outdated
| if ((length <= 1 + sizeof(uint32_t)) || m_stopState != ESCARGOT_DEBUGGER_IN_WAIT_MODE) { | ||
| break; | ||
| } | ||
| m_watchEval = true; |
There was a problem hiding this comment.
The assignment to m_watchEval = true is not reset if doEval throws, leaving the debugger in an inconsistent state. Wrap the call in a try/finally or use a guard to reset m_watchEval to false on all exit paths.
matetokodi
force-pushed
the
fix_debugger_multiple_watches
branch
from
baf7bd8 to
61f6c4b
Compare
src/debugger/Debugger.cpp
Outdated
| } | ||
|
|
||
| m_stopState = ESCARGOT_DEBUGGER_IN_WAIT_MODE; | ||
| if (m_watchEval) { |
There was a problem hiding this comment.
Replace the if (m_watchEval) { m_watchEval = false; } block with a direct assignment m_watchEval = false;. This removes an unnecessary branch, simplifying control flow. Setting the flag to false when it is already false has no effect, so the change is semantics‑preserving. The refactor improves readability and maintainability without altering behavior.
| m_stopState = stopState; | ||
| return false; | ||
| } | ||
| case ESCARGOT_MESSAGE_WATCH_8BIT_START: |
There was a problem hiding this comment.
The added case ESCARGOT_MESSAGE_WATCH_8BIT_START: label has no associated code block, causing the watch message to fall through to the next case and be handled as an eval. This logic error means watch evaluations are not performed correctly. Add the original block or remove the case to prevent unintended eval handling.
Processing VSCode watches skips waitForResolvingPendingBreakpoints (previously having multiple watches in VSC would call this when processing the first watch, which would process pending messages while trying to wait for break point changes while in eval mode (not in waiting mode) and cause and Invalid message error and the closure of the debgger connection when hitting the second watch message) Allow '*.mjs' files in the python debugger Signed-off-by: Máté Tokodi mate.tokodi@szteszoftver.hu
matetokodi
force-pushed
the
fix_debugger_multiple_watches
branch
from
61f6c4b to
81b6907
Compare
|
Can you make testcase? |
| ESCARGOT_DEBUGGER_WAIT_BEFORE_EXIT = 25 | ||
| ESCARGOT_DEBUGGER_STOP = 26 | ||
| ESCARGOT_MESSAGE_WATCH_8BIT_START = 27 | ||
| ESCARGOT_MESSAGE_WATCH_8BIT = 28 |
There was a problem hiding this comment.
I think 16 bit variants needs to be added as well
3 participants
Processing VSCode watches skips
waitForResolvingPendingBreakpoints(previously having multiple watches in VSC would call this when processing the first watch, which would process pending messages while trying to wait for break point changes while in eval mode (not in waiting mode) and cause and Invalid message error and the closure of the debgger connection when hitting the second watch message)Allow '*.mjs' files in the python debugger
In combination with: Samsung/escargot-vscode-extension#28