Bugfix: KMail: Detach from message observation when destructing KMReaderWin #23

When opening and closing new main-windows, failure to detach from ISubjects at KMReaderWin destruction can result in a heap-use-after-free segfault when the ISubject tries to notify the (now non-existent) KMReaderWin of an event.

==26775==ERROR: AddressSanitizer: heap-use-after-free on address 0x61a00008a758 at pc 0x7f0f082d4fe4 bp 0x7ffd111e8740 sp 0x7ffd111e8730
READ of size 8 at 0x61a00008a758 thread T0
    #0 0x7f0f082d4fe3 in KMail::ISubject::notify() /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/isubject.cpp:39
    #1 0x7f0f07b33f94 in KMMessage::updateBodyPart(TQString, TQMemArray<char> const&) /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/kmmessage.cpp:4347
    #2 0x7f0f0824bd54 in KMail::ImapJob::slotGetMessageResult(TDEIO::Job*) /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/imapjob.cpp:417
    #3 0x7f0f0825196a in KMail::ImapJob::tqt_invoke(int, TQUObject*) /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999_build/kmail/imapjob.moc:136
    #4 0x7f0f061bebb7 in TQObject::activate_signal(TQConnectionList*, TQUObject*) kernel/qobject.cpp:2813
    #5 0x7f0f0035602e in TDEIO::Job::result(TDEIO::Job*) /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999_build/tdeio/tdeio/jobclasses.moc:173
    #6 0x7f0f003562c5 in TDEIO::Job::emitResult() /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/job.cpp:235
    #7 0x7f0f0037b4a4 in TDEIO::SimpleJob::slotFinished() /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/job.cpp:601
    #8 0x7f0f0037f7c6 in TDEIO::TransferJob::slotFinished() /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/job.cpp:1006
    #9 0x7f0f003aa5d9 in TDEIO::TransferJob::tqt_invoke(int, TQUObject*) /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999_build/tdeio/tdeio/jobclasses.moc:1156
    #10 0x7f0f061bebb7 in TQObject::activate_signal(TQConnectionList*, TQUObject*) kernel/qobject.cpp:2813
    #11 0x7f0f061bed9c in TQObject::activate_signal(int) kernel/qobject.cpp:2747
    #12 0x7f0f00328f94 in TDEIO::SlaveInterface::dispatch(int, TQMemArray<char> const&) /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/slaveinterface.cpp:243
    #13 0x7f0f00320651 in TDEIO::SlaveInterface::dispatch() /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/slaveinterface.cpp:173
    #14 0x7f0f0031ae1b in TDEIO::Slave::gotInput() /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/slave.cpp:300
    #15 0x7f0f0031ff6e in TDEIO::Slave::tqt_invoke(int, TQUObject*) /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999_build/tdeio/tdeio/slave.moc:124
    #16 0x7f0f061bebb7 in TQObject::activate_signal(TQConnectionList*, TQUObject*) kernel/qobject.cpp:2813
    #17 0x7f0f061beefc in TQObject::activate_signal(int, int) kernel/qobject.cpp:2977
    #18 0x7f0f061e0631 in TQSocketNotifier::event(TQEvent*) kernel/qsocketnotifier.cpp:261
    #19 0x7f0f06160e6c in TQApplication::internalNotify(TQObject*, TQEvent*) kernel/qapplication.cpp:2883
    #20 0x7f0f061614b6 in TQApplication::notify(TQObject*, TQEvent*) kernel/qapplication.cpp:2726
    #21 0x7f0f06d9aa6c in TDEApplication::notify(TQObject*, TQEvent*) /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdecore/tdeapplication.cpp:660
    #22 0x7f0f06156707 in TQEventLoop::activateSocketNotifiers() kernel/qeventloop_unix.cpp:588
    #23 0x7f0f06140c11 in TQEventLoop::processEvents(unsigned int) kernel/qeventloop_x11.cpp:390
    #24 0x7f0f06177dde in TQEventLoop::enterLoop() kernel/qeventloop.cpp:227
    #25 0x7f0f06177d21 in TQEventLoop::exec() kernel/qeventloop.cpp:174
    #26 0x556e25d09bb8 in main (/usr/trinity/14/bin/kmail+0x3bb8)
    #27 0x7f0f056a5850 in __libc_start_main ../csu/libc-start.c:308
    #28 0x556e25d09d79 in _start (/usr/trinity/14/bin/kmail+0x3d79)

0x61a00008a758 is located 216 bytes inside of 1208-byte region [0x61a00008a680,0x61a00008ab38)
freed by thread T0 here:
    #0 0x7f0f08cd7ef7 in operator delete(void*, unsigned long) /var/tmp/portage/sys-devel/gcc-8.2.0-r6/work/gcc-8.2.0/libsanitizer/asan/asan_new_delete.cc:151
    #1 0x7f0f081ce68c in KMMainWidget::destruct() /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/kmmainwidget.cpp:282

previously allocated by thread T0 here:
    #0 0x7f0f08cd6557 in operator new(unsigned long) /var/tmp/portage/sys-devel/gcc-8.2.0-r6/work/gcc-8.2.0/libsanitizer/asan/asan_new_delete.cc:90
    #1 0x7f0f0821200f in KMMainWidget::createWidgets() /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/kmmainwidget.cpp:635
    #2 0x7f0f08218d6e in KMMainWidget::KMMainWidget(TQWidget*, char const*, KXMLGUIClient*, TDEActionCollection*, TDEConfig*) /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/kmmainwidget.cpp:199

When opening and closing new main-windows, failure to detach from ISubjects at KMReaderWin destruction can result in a heap-use-after-free segfault when the ISubject tries to notify the (now non-existent) KMReaderWin of an event. ``` ==26775==ERROR: AddressSanitizer: heap-use-after-free on address 0x61a00008a758 at pc 0x7f0f082d4fe4 bp 0x7ffd111e8740 sp 0x7ffd111e8730 READ of size 8 at 0x61a00008a758 thread T0 #0 0x7f0f082d4fe3 in KMail::ISubject::notify() /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/isubject.cpp:39 #1 0x7f0f07b33f94 in KMMessage::updateBodyPart(TQString, TQMemArray<char> const&) /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/kmmessage.cpp:4347 #2 0x7f0f0824bd54 in KMail::ImapJob::slotGetMessageResult(TDEIO::Job*) /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/imapjob.cpp:417 #3 0x7f0f0825196a in KMail::ImapJob::tqt_invoke(int, TQUObject*) /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999_build/kmail/imapjob.moc:136 #4 0x7f0f061bebb7 in TQObject::activate_signal(TQConnectionList*, TQUObject*) kernel/qobject.cpp:2813 #5 0x7f0f0035602e in TDEIO::Job::result(TDEIO::Job*) /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999_build/tdeio/tdeio/jobclasses.moc:173 #6 0x7f0f003562c5 in TDEIO::Job::emitResult() /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/job.cpp:235 #7 0x7f0f0037b4a4 in TDEIO::SimpleJob::slotFinished() /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/job.cpp:601 #8 0x7f0f0037f7c6 in TDEIO::TransferJob::slotFinished() /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/job.cpp:1006 #9 0x7f0f003aa5d9 in TDEIO::TransferJob::tqt_invoke(int, TQUObject*) /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999_build/tdeio/tdeio/jobclasses.moc:1156 #10 0x7f0f061bebb7 in TQObject::activate_signal(TQConnectionList*, TQUObject*) kernel/qobject.cpp:2813 #11 0x7f0f061bed9c in TQObject::activate_signal(int) kernel/qobject.cpp:2747 #12 0x7f0f00328f94 in TDEIO::SlaveInterface::dispatch(int, TQMemArray<char> const&) /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/slaveinterface.cpp:243 #13 0x7f0f00320651 in TDEIO::SlaveInterface::dispatch() /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/slaveinterface.cpp:173 #14 0x7f0f0031ae1b in TDEIO::Slave::gotInput() /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdeio/tdeio/slave.cpp:300 #15 0x7f0f0031ff6e in TDEIO::Slave::tqt_invoke(int, TQUObject*) /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999_build/tdeio/tdeio/slave.moc:124 #16 0x7f0f061bebb7 in TQObject::activate_signal(TQConnectionList*, TQUObject*) kernel/qobject.cpp:2813 #17 0x7f0f061beefc in TQObject::activate_signal(int, int) kernel/qobject.cpp:2977 #18 0x7f0f061e0631 in TQSocketNotifier::event(TQEvent*) kernel/qsocketnotifier.cpp:261 #19 0x7f0f06160e6c in TQApplication::internalNotify(TQObject*, TQEvent*) kernel/qapplication.cpp:2883 #20 0x7f0f061614b6 in TQApplication::notify(TQObject*, TQEvent*) kernel/qapplication.cpp:2726 #21 0x7f0f06d9aa6c in TDEApplication::notify(TQObject*, TQEvent*) /var/tmp/portage/trinity-base/tdelibs-9999/work/tdelibs-9999/tdecore/tdeapplication.cpp:660 #22 0x7f0f06156707 in TQEventLoop::activateSocketNotifiers() kernel/qeventloop_unix.cpp:588 #23 0x7f0f06140c11 in TQEventLoop::processEvents(unsigned int) kernel/qeventloop_x11.cpp:390 #24 0x7f0f06177dde in TQEventLoop::enterLoop() kernel/qeventloop.cpp:227 #25 0x7f0f06177d21 in TQEventLoop::exec() kernel/qeventloop.cpp:174 #26 0x556e25d09bb8 in main (/usr/trinity/14/bin/kmail+0x3bb8) #27 0x7f0f056a5850 in __libc_start_main ../csu/libc-start.c:308 #28 0x556e25d09d79 in _start (/usr/trinity/14/bin/kmail+0x3d79) 0x61a00008a758 is located 216 bytes inside of 1208-byte region [0x61a00008a680,0x61a00008ab38) freed by thread T0 here: #0 0x7f0f08cd7ef7 in operator delete(void*, unsigned long) /var/tmp/portage/sys-devel/gcc-8.2.0-r6/work/gcc-8.2.0/libsanitizer/asan/asan_new_delete.cc:151 #1 0x7f0f081ce68c in KMMainWidget::destruct() /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/kmmainwidget.cpp:282 previously allocated by thread T0 here: #0 0x7f0f08cd6557 in operator new(unsigned long) /var/tmp/portage/sys-devel/gcc-8.2.0-r6/work/gcc-8.2.0/libsanitizer/asan/asan_new_delete.cc:90 #1 0x7f0f0821200f in KMMainWidget::createWidgets() /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/kmmainwidget.cpp:635 #2 0x7f0f08218d6e in KMMainWidget::KMMainWidget(TQWidget*, char const*, KXMLGUIClient*, TDEActionCollection*, TDEConfig*) /var/tmp/portage/trinity-base/kmail-9999/work/kmail-9999/kmail/kmmainwidget.cpp:199 ```

It looks good. I just ask for a small formatting style adjustment.

Hi Luke, the fix looks neat and simple, thanks! Is there a way to reproduce the original problem systematically? I would like to test here too, but I have not been able to crash Kmail so far 😞

I have added the parenthesis to Luke's code and credited Luke's for the commit anyway.

PR merged to main code and cherry-picked into R14.0.x branch as well.

Thanks for the good work Luke!

It typically happened when I made a new main window and then immediately closed the old main window.

(I did this because Xpra likes to "freeze" the window contents, and making a new window works around that.)

ok, thanks for the info Luke. Keep up the good work, we are glad to see new contributors 👍

 {
   if (message())
     message()->detach( this );