[Okular-devel] Review Request 125890: Fix crash on close

Alex Merry alex.merry at kde.org
Wed Nov 4 22:48:10 UTC 2015



> On Nov. 2, 2015, 9:52 p.m., Albert Astals Cid wrote:
> > I'd prefer to know who decides to set a tab of -1 since this doesn't seem to be needed in the non frameworks branch seems like the code is "breaking" somewhere else
> 
> David Rosca wrote:
>     QTabWidget::currentChanged probably because the tab widget (KPart widget) is destroyed before QTabWidget. 
>     
>     delete m_tabWidget in Shell destructor works too.
> 
> Albert Astals Cid wrote:
>     That works for me.
> 
> Albert Astals Cid wrote:
>     I mean i prefer the delete, sorry if was unclear.

I suspect the reason you're seeing this now and didn't before is https://quickgit.kde.org/?p=kparts.git&a=commit&h=b72fc5e56579035bf987075e16324ef95ef8e3d4 - note that Okular's old behaviour was wrong (clearing m_tabs while signals were still connected to slots that used it), but KPart's destructor chain used to hide that.

That said, I'm not convinced the delete is in the right place in this change. I think deleting the widget before clearing m_tabs is still asking for trouble (even if it seems to fix it).

The fix I had lined up for this issue was to do

    m_tabWidget->disconnect(this);

before clearing m_tabs.


- Alex


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


On Nov. 4, 2015, 11:18 a.m., David Rosca wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125890/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2015, 11:18 a.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> Delete m_tabwidget in destructor
> 
> 
> Diffs
> -----
> 
>   shell/shell.cpp ce5e76c 
> 
> Diff: https://git.reviewboard.kde.org/r/125890/diff/
> 
> 
> Testing
> -------
> 
> No longer crashes.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20151104/3e9b8384/attachment.html>


More information about the Okular-devel mailing list