replace custom backtracing in SlaveBase with KCrash
ClosedPublic

Authored by sitter on Sep 17 2018, 12:26 PM.

Details

Summary

as discussed on mailing list.

KCrash originally was automatically initialized, but now needs to be
explicitly linked to work.
additionally the custom backtrace() in the SlaveBase is only marginally
useful and largely replaced by improvements to KCrash (such as core pattern
support on linux to allow raising crashes to a global core collector such
as systemd's coredumpd).

If someone wants to resurrect the custom backtrace() feature it needs to
be moved to KCrash and actually made signal-safe (explicitly load libgcc;
see backtrace manpage).

CHANGELOG: KIO slave crashes are now handled by KCrash instead of subpar custom code

Test Plan
  • q_assert(false) in a slave now brings up drkonqi

Diff Detail

Repository
R241 KIO
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter created this revision.Sep 17 2018, 12:26 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptSep 17 2018, 12:26 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
sitter requested review of this revision.Sep 17 2018, 12:26 PM
dfaure accepted this revision.Sep 17 2018, 5:07 PM
This revision is now accepted and ready to land.Sep 17 2018, 5:07 PM
This revision was automatically updated to reflect the committed changes.
cfeck added a subscriber: cfeck.Jan 4 2019, 3:51 AM

This seems to pull in QtGui dependency, but that's also dragged in by KService (which also links to KF5::Crash) and KF5DBusAddons (which only needs QtX11Extras, which unfortunately also needs QtGui).

Can the KCrash dependency be added to the slaves instead of to the KIOCore library? The split into Core/Gui/Widgets got a bit void when linking an application against KIOCore effectively drags in QtGui and QtWidgets (I have yet to check why QtWidgets is dragged in).

I am somewhat indifferent. It certainly has caused some grief for various reasons in various places, so it stands to reason that it should go. Also since auto-init of kcrash is essentially not a thing for application code (what with link-as-needed being a common distro flag) it's possibly more consistent to move it's enabling to the individual slaves.

OTOH the forced kcrashing did reveal a whole bunch of problems with the existing slaves. It's not all bad. Perhaps the way to go is to detangle kcrash from qtgui as a hard dep. As I recall it's only used for reinit-tech anyway, so hardly useful for the slave use-case. kcrashcore if you will ;) (I doubt we'd actually need a separate lib to accomplish it though)