Review Request 127441: Fix never ending event loop when quitting app and segmentation faults

[Andreas Cord-Landwehr]

> On März 21, 2016, 7:50 vorm., Andreas Cord-Landwehr wrote:
> > src/parleydocument.cpp, line 92
> > <https://git.reviewboard.kde.org/r/127441/diff/1/?file=453667#file453667line92>
> >
> >     Don't we get a resource leak now?
> 
> Hartmut Riesenbeck wrote:
>     The resorces were already realeased in the close() method one line before. There the m_doc pointer is also set to zero. Which for my understanding caused the segmentation fault by calling m_doc->deleteLater() with m_doc=0.
>     The m_backupTimer doesn't need to be deleted here, since it is created (if needed) as a child of the recent ParleyDocument object.

Thanks, missed that line.


> On März 21, 2016, 7:50 vorm., Andreas Cord-Landwehr wrote:
> > src/parleymainwindow.cpp, lines 132-133
> > <https://git.reviewboard.kde.org/r/127441/diff/1/?file=453668#file453668line132>
> >
> >     I do not see why is solves the problem. If possible, we always prefer the function pointer ways, since it is faster and ensures that the call is correct at compile time. (Neglecting corner cases, the only reason why one sometimes still uses the old mechanism is when one creates QML bindings.)
> 
> Hartmut Riesenbeck wrote:
>     You'are right, the SIGNAL/SLOT macros aren't the cause of the problem. Since I just started with qt5, I didn't know this way to connect signals. My intention here is to use the provided slot argument instead of the document() method. In the course of this change, I corrected the rest of the line in the way I thought it was better (with my outdated qt4 knowledge). Now after reading the qt5 dos, I agree that the function pointer way is better to use. I will upload a corrected diff this evening.
>     
>     By the the way. In line 134 - is it better to use:
>     
>             connect(doc, &QObject::destroyed, this, &ParleyMainWindow::slotUpdateWindowCaption);
>     or
>             connect(doc, &KEduVocDocument::destroyed, this, &ParleyMainWindow::slotUpdateWindowCaption);  ?
>     
>     Both versions compiles and runs without error. The second one may better documents that desroying of of the EduVocDocument ist meant.

Yes, I would also go this the second way because it documents the meaning better. Performance-wise that shouldn't make any difference.


- Andreas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127441/#review93808
-----------------------------------------------------------


On März 20, 2016, 7:49 nachm., Hartmut Riesenbeck wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127441/
> -----------------------------------------------------------
> 
> (Updated März 20, 2016, 7:49 nachm.)
> 
> 
> Review request for KDE Edu.
> 
> 
> Repository: parley
> 
> 
> Description
> -------
> 
> When the app is quited app.exec() doesn't return. The process stays in
> sleeping state. Removed app.setQuitOnLastWindowClosed(false) because
> the quit() or exec() method of the application were not invoked
> somewhere else in the application.
> 
> After this the app ends in the correct way. But after quit is invoked,
> there are sporadic segmentation faults and the following message was
> shown on stderr:
>     QCoreApplication::postEvent: Unexpected null receiver
> This was fixed by removing zero pointer access in destuctor of
> ParleyDocument and adding SIGNAL SLOT macros into connect statement in
> ParleyMainWidow.
> 
> 
> Diffs
> -----
> 
>   src/main.cpp 2e87baa93f81349a187d620be2468f8a1bb4aca1 
>   src/parleydocument.cpp cf4c81c3f8f35a99389d61a75d934cff3170b960 
>   src/parleymainwindow.cpp d6f86debd32c1e77bbcca86a160906674960dce9 
> 
> Diff: https://git.reviewboard.kde.org/r/127441/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hartmut Riesenbeck
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20160322/db6b62b8/attachment.html>