D15295: [konsole]: proper fix for a crash-on-exit

[René J.V. Bertin]

rjvbb created this revision.
rjvbb added reviewers: Konsole, gateau.
rjvbb added a project: Konsole.
Herald added a subscriber: konsole-devel.
rjvbb requested review of this revision.

REVISION SUMMARY
  An old commit (dd1b2b4d <https://phabricator.kde.org/R319:dd1b2b4df04f13bb2a9f3bcef106dc3604c5fc1a>) addressed a crash-on-exit (not well understood) by reverting to the old-style signal/slot connection syntax.
  
  In practice, this prevented simply because the slot was never called, something I got wind when after installed the current tip of the master branch I started seeing this:
  
    QObject::connect: No such signal Konsole::TabbedViewContainer::destroyed(TabbedViewContainer*)
  
  Moving back to the new style syntax (after changing the signature of ` ViewSplitter::containerDestroyed` to avoid having to use a complicated cast in the connect() call) the crash was indeed there anew because containerDestroyed was being called again.
  Analysing the backtrace showed that the containerDestroyed slot was called under the QSplitter::~QSplitter dtor, while deleting the splitter's child widgets.
  
  In other words, the crash (inside QList::removeAll) seemed due to removing an item from a stale QList object. Vieplitter::containerDestroyed only serves to unregister a TabbedViewContainer that is about to be deleted; indeed, unregistering all registered containers in the new ViewSplitter::~ViewSplitter dtor fixes the crash.
  
  My patch reverts both connections in ViewSplitter::registerContainer() to the new-style connection syntax.

TEST PLAN
  Without the patch: no crash on exit because TabbedViewContainers aren't being unregistered at all
  With the patch: no crash on exit because they are unregistered while that is still possible.
  
  I have not been able to test runtime invocation of ViewSplitter::containerDestroyed. I did notice that the qobject_cast in the patched slot fails if I do NOT emit the destroyed signal explicitly from the TabbedViewContainer dtor.
  
  I do wonder about the remainder of the containerDestroyed function: will `count()` already be 0 when the last container is *about* to be deleted?

REPOSITORY
  R319 Konsole

REVISION DETAIL
  https://phabricator.kde.org/D15295

AFFECTED FILES
  src/ViewSplitter.cpp
  src/ViewSplitter.h

To: rjvbb, #konsole, gateau
Cc: konsole-devel, #kde_applications, herrold, ngraham, maximilianocuria, hindenburg
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20180905/6b5d98c8/attachment.html>