D12371: fix always reproducible crash

David Faure noreply at phabricator.kde.org
Fri Apr 20 17:21:54 UTC 2018


dfaure requested changes to this revision.
dfaure added a comment.


  I find this solution horrible. QPointers screams "we designed this wrongly, the ownership rules are unclear, stuff gets deleted randomly, we have to guard against it".
  
  So while I don't deny that we might have designed this wrongly (and if we did, it's probably my fault), the right solution is to design this correctly, not to sprinkle qpointers.
  
  Changing the documented ownership (as in the first version of the patch) isn't a solution either (that's behaviour incompatible by definition), but a standard solution is setAutoDeleteDirLister(false) for the case where you don't want the ownership to be transferred.
  
  I don't understand "KCoreDirListerCache is probably the owner." People can create a KDirLister directly.
  
  Also, the commit description reads like "kdirlister got deleted, KCoreDirListerCache wasn't told", which is surprising since the KCoreDirLister destructor deregisters from the cache using forgetDirs(this), which ends up calling dirData.listersCurrentlyHolding.removeAll(lister);
  So I think the investigation hasn't been completed, which then leads to a bad fix...

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks, apol
Cc: apol, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180420/45525933/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list