Review Request 128227: Don't hold any lock when QML/JS calls qmlplugindump

[Milian Wolff]

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


Fix it, then Ship it!




one small nitpick, otherwise lgtm


languages/qmljs/duchain/cache.cpp (line 137)
<https://git.reviewboard.kde.org/r/128227/#comment65308>

    this should reuse cachedFilePath here


- Milian Wolff


On June 17, 2016, 4:51 p.m., Denis Steckelmacher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128227/
> -----------------------------------------------------------
> 
> (Updated June 17, 2016, 4:51 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Bugs: 363509
>     http://bugs.kde.org/show_bug.cgi?id=363509
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> The QML/JS language plugin contains a cache that stores information about known plugin paths, locations of imported files and binary QML modules. When the user imports a binary QML module, qmlplugindump is called in order to list its contents.
> 
> This patch slightly reworks how a lock is held and released in order to ensure that no lock is held while calling qmlplugindump. In some cases (bug 363509), it seems that qmlplugindump can hang or take a very long time to respond. If Cache is locked during that time, no other thread can parse any QML file. It may also be possible that other threads are blocked while the global DUChain lock is held, which may completely freeze parsing in any language.
> 
> 
> Diffs
> -----
> 
>   languages/qmljs/duchain/cache.cpp fe63586 
> 
> Diff: https://git.reviewboard.kde.org/r/128227/diff/
> 
> 
> Testing
> -------
> 
> The QML/JS testsuite still runs and passes. I was not able to reproduce the bug, so I encourage anyone having faced the problem to test this patch.
> 
> 
> Thanks,
> 
> Denis Steckelmacher
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20160617/01b8080c/attachment.html>