Fix Xdebug disconnecting after php finished if multiple connections are allowed
ClosedPublic

Authored by wcancino on Jan 9 2019, 2:16 PM.

Details

Summary
  • If we launch a php xdebug process (browser) multiple connections are supported by default. If the debugged script finishes, the debug session do not keep

listening for additional connections.

  • Various code fixes and some refactorization.
Test Plan
  • Launch a php script for debug (with some breakpoint defined).
  • When script finishes, reload the url(on the browser), php debugger will accept the connection and start debugging the script

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.Jan 9 2019, 2:16 PM
Restricted Application added a project: KDevelop. · View Herald TranscriptJan 9 2019, 2:16 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
wcancino requested review of this revision.Jan 9 2019, 2:16 PM
brauch added a subscriber: pprkut.Jan 9 2019, 2:33 PM

@pprkut can you look at this? I have not used PHP or its tooling for more than 10 years ...

pprkut added a comment.Jan 9 2019, 7:46 PM

I'm afraid I don't know much about this either :-/
The fact that the unit tests are currently completely broken doesn't help. I did try them with and without this change and what I *can* say is that it's neither worse nor better.

Since this fixes an actual issue in code that seems to be unit tested afaics, I think adding unit tests would be appreciated. But that would require fixing the current ones first :-/

I'm willing to give this the benefit of the doubt, but wonder if taking this without tests will make it harder to eventually fix them.

Well, it is true that this plugins dates from the Kdev4 days, and it is unmaintained. Anyways, with these patch, I correct the plugin to behave as described on

http://nikosams.blogspot.com/2010/02/kdevelop-xdebug-php-debugger.html (section 2)

It is also correct some crashes, due to the fact (in my opinion) that this plugins treat to map the "connection" state to the "session state" and it doesn't map

very well. The way kdevelop debugger infrastructure manages the states make that when a debug session passes to 'NotStartedState' the 'DebugLaunch/Continue'

button (in 'Debug Launch' mode) is clickable and it immediately invokes to the run method. In the Xdebug case, a connection is active while a program is running but when the

script stops, the connection stopped as well. At this moment, it is logical the debug session ends at the same time, but Xdebug plugin allows to continue

to "listen" for future incoming scripts, so session is still alive and passed to NotStartedState. Doing so, the aforementioned Debug Launch button activates

and if it is pressed invokes to the 'run' method, but there is no script to debug ( nor connection ) and that causes troubles. This patch avoids it, and as well

fixes some corner cases, where if the launch of the debugger fails, it ended properly the session (otherwhise, session remains alive in 'notstartedstate' and debug launch button activates )

I try another debugger (python and c++) and in both cases, I was not able to see the 'Debug launch' button active (just 'Continue'), that let me curious about the

intention of that aforementioned 'Debug launch'.

Moreover, I do not see the tests cases so broken, I will try to find time to fix the multiple connection test.

wcancino updated this revision to Diff 49799.Jan 18 2019, 10:11 AM
  • Fix testcase
mwolff accepted this revision.Jan 26 2019, 11:09 PM
mwolff added a subscriber: mwolff.

since noone else is using this plugin, I'd say let's push this! @wcancino do you have a developer account? if not, then we can commit this for you. Please then give us your email address to associate with the commit then.

This revision is now accepted and ready to land.Jan 26 2019, 11:09 PM

Hello all, No I do not have dev account, you can associate this patch to
wcancino@gmail.com

Regards,

Le dim. 27 janv. 2019 à 00:09, Milian Wolff <noreply@phabricator.kde.org> a
écrit :

mwolff accepted this revision.
mwolff added a comment.
This revision is now accepted and ready to land. View Revision
https://phabricator.kde.org/D18122

since noone else is using this plugin, I'd say let's push this! @wcancino
https://phabricator.kde.org/p/wcancino/ do you have a developer
account? if not, then we can commit this for you. Please then give us your
email address to associate with the commit then.

*REPOSITORY*
R68 KDevelop XDebug Support

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

*To: *wcancino, brauch, pprkut, mwolff
*Cc: *mwolff, pprkut, kdevelop-devel, glebaccon, hase, antismap, iodelay,
geetamc, Pilzschaf, akshaydeo, surgenight, arrowd

This revision was automatically updated to reflect the committed changes.