Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix. Set/Del breakpoints may not work in some cases. #68

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

moteus
Copy link
Contributor

@moteus moteus commented Jan 8, 2022

Problem that if buf has partial line e.g. SETB fi the mobdebug will not recognize it as a set breakpoint command.

Problem that if buf has partial line e.g. `SETB fi` the mobdebug will not recognize it as a set breakpoint command.
src/mobdebug.lua Show resolved Hide resolved
@pkulchenko
Copy link
Owner

Thank you for the patch; I agree there may be a problem with a partial buffer longer than 5 bytes, but do you have a test case or a description where this may happen?

@moteus
Copy link
Contributor Author

moteus commented Jan 8, 2022

do you have a test case or a description where this may happen?

No. I just read the code :)

@pkulchenko
Copy link
Owner

No. I just read the code :)

I sometimes do this too ;)

I wonder if this is the issue that may be causing pkulchenko/ZeroBraneStudio#909 (as that issue seems to be related to un/setting async breakpoints, which is what this fragment handles):

...it looks like it happens mostly when the Stack window is shown. So, the application is stopped, asetb command is triggered, but the stack is being retrieved first and the result of the stack command gets mixed with the asetb command.

@pkulchenko
Copy link
Owner

@moteus, I was re-reading the current logic and don't think it should cause this issue, as is_pending(server) check only reads 1 byte and only if the buf value is nil, so I don't see a scenario for it to get into having a partial buffer with 5 characters.

If you suspect that SETB fi may be accepted, then I think it shouldn't pass line 525 with the partial check. Yes, it seems like it may get into a situation when SETB fi partial is stored in the buffer, which will attempt readnext with a negative number (on line 521), but it should trigger a runtime error. Hm, I wonder if I can reproduce this and if this may cause some leftovers in the buffer that can "fuse" with the next command...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants