search

pkulchenko / MobDebug

Remote debugger for Lua.
Other
889 stars 192 forks source link

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

Open moteus opened 2 years ago

moteus commented 2 years ago

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

pkulchenko commented 2 years ago

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 commented 2 years ago

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

No. I just read the code :)

pkulchenko commented 2 years ago

No. I just read the code :)

I sometimes do this too ;)

I wonder if this is the issue that may be causing https://github.com/pkulchenko/ZeroBraneStudio/issues/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 commented 2 years ago

@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...