Do not crash if there is still pending connections
ClosedPublic

Authored by wcancino on Nov 28 2017, 1:48 PM.

Details

Summary

If we put the debugger on a "ended stated" even if there are still pending connections, kdevelop will crash.

Diff Detail

Repository
R68 KDevelop XDebug Support
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
wcancino created this revision.Nov 28 2017, 1:48 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptNov 28 2017, 1:48 PM
wcancino requested review of this revision.Nov 28 2017, 1:48 PM
brauch added a subscriber: brauch.Nov 28 2017, 2:00 PM

I think I hit this issue yesterday when testing kdev-xdebug, esp. the debugger wouldn't stop properly when stepping over the end of the program. I don't quite understand what issue exactly this change fixes and why, can you write a few lines?

wcancino added a comment.EditedNov 28 2017, 2:09 PM

As far I remember, the server will close even if they are listening connections, so next time we start xdebug, it will crash. It was mostly trial and error approach.

Hm yes, ok, so it makes sense why this fixes the crash. I think we can keep this as a safety guard. However I think the actual problem is that the state is not set to EndedState when the debug session actually ends, which also has other undesirable consequences (e.g. KDevelop doesn't switch back to code view, etc.). Do you have an idea why this happens?

wcancino added a comment.EditedNov 28 2017, 2:19 PM

Well, I think that main source of problems is that Xdebug plugins maps the connection state to the debug state, and it generates a bunch of side effects if something get wrong. I think no other debug plugin do the same. So, for instance, if you do not have a connection it is necessary to set the status "manually" as I did in another patch.

Ok, but can we not at least set the state to Ended when the connection breaks, or something like that?

Well, I did some experiments and I think I got it why it was coded like that, and why the patch is correct (depending on what behavoiur was intended by original author):

IMHO, the original intention was for the following case:

  • Let start the debug of some php script,
  • As multiconnection is allowed, another instance of such script can be launched waiting to be debugged.
  • As soon as the first script finished, debugger enter to NonStarted state and he second script can be debugged ( and it will stops on the break points defined).

The problem was the debugger reamined in NonStarted state even if pending connections. Side effect, "Debug launch" button is enable, as "NonStarted"
state allows it, and then Kdevelop crashed.

We can fixed as well if we put a EndedState for any case, but in that case, if a second connection is waiting, it will simply not debug.

brauch accepted this revision.Nov 29 2017, 10:43 AM

Alright, if you think it makes sense, submit it. It's certainly an improvement over the old situation. Thanks!

This revision is now accepted and ready to land.Nov 29 2017, 10:43 AM
This revision was automatically updated to reflect the committed changes.

Hello Sven,

Could you take a look at D18122

Regards,

Waldo

Le mar. 28 nov. 2017 à 15:00, Sven Brauch <noreply@phabricator.kde.org> a
écrit :

brauch added a comment. View Revision https://phabricator.kde.org/D9034

I think I hit this issue yesterday when testing kdev-xdebug, esp. the
debugger wouldn't stop properly when stepping over the end of the program.
I don't quite understand what issue exactly this change fixes and why, can
you write a few lines?

*REPOSITORY*
R68 KDevelop XDebug Support

*REVISION DETAIL*
https://phabricator.kde.org/D9034

*To: *wcancino, kfunk
*Cc: *brauch, kdevelop-devel