Review Request 126955: Fix crash in activities context menu

[David Faure]

> On Feb. 2, 2016, 7:49 a.m., David Faure wrote:
> > Looks correct, although IMHO it's the code that creates a class, that should decide how it should get deleted, rather than the class itself deciding that (same reason why for widgets, WA_DeleteOnClose should be set after new, not in the ctor).
> 
> David Edmundson wrote:
>     In this case it looks like it's meant to be a KJob style pattern. It does an emit(result) just before the original deleteLater()
>     
>     I didn't really want to change the behaviour, but I'm happy to if that's something wanted.
> 
> Ivan Čukić wrote:
>     You can create a static method that creates it so that we do not have a new-without-delete. I guess it is the easiest way to keep the logic and not have the isssue David mentioned.

Ah, that's why KIO has KIO::get().... I never really realized. But yeah, since then we have "new Job" for many things in many places indeed (akonadi, etc.). Jobs have a setAutoDelete(false) though, in case someone wants to get control over the lifetime again. Anyhow, I withdraw my objection, no big deal.


- David


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


On Feb. 1, 2016, 9:16 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126955/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2016, 9:16 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 351485
>     http://bugs.kde.org/show_bug.cgi?id=351485
> 
> 
> Repository: kactivities
> 
> 
> Description
> -------
> 
> Currently we call deleteLater() from inside ::run which is running in a
> different thread than the receiving object.
> (QThread objects live in the thread that created them, not in the thread they create) 
> 
> This patch causes deleteLater to be run in the right thread.
> 
> QCoreApplication::postEvent is thread safe but it needs to be in the right thread to work out the correct event loop level for deferred delete events.
> 
> BUG: 351485
> 
> 
> Diffs
> -----
> 
>   src/workspace/fileitemplugin/FileItemLinkingPluginActionLoader.cpp 3343eb4c5cfe209e20b0210a2b7fdf980a1e8b4a 
> 
> Diff: https://git.reviewboard.kde.org/r/126955/diff/
> 
> 
> Testing
> -------
> 
> Right click in dolpin. Still works.
> 
> Couldn't reproduce the original crash, so I don't know for sure it fixes it. 
> Debug in qcoreapplication showed we got a different eventLoopLevel on the QDeferredDeleteEvent
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160202/b51ef413/attachment.html>