Review Request 118817: Ensure we never access a given CXTranslationUnit concurrently.

[Kevin Funk]

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

Ship it!


I like the approach. Makes it impossible to access the TU without acquiring the lock first (because you must instantiate a ParseSession object in order to get a hold of CXTranslationUnit from ParseSessionData).

Fixes the assert for me and nothing broke as far as I can see.

- Kevin Funk


On June 18, 2014, 6:10 p.m., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118817/
> -----------------------------------------------------------
> 
> (Updated June 18, 2014, 6:10 p.m.)
> 
> 
> Review request for KDevelop, Kevin Funk and Olivier Jean de Gaalon.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> Ensure we never access a given CXTranslationUnit concurrently.
> 
> ParseSession's data is moved into ParseSessionData, which is the
> actual IAstContainer implementation. It also now stores a QMutex.
> This mutex is implicitly locked inside ParseSession, so you can say
> a ParseSession is now similar to a QMutexLocker, with the difference
> that you can exchange the QMutex it operates on. This simplifies the
> code in the parse job in my opinion.
> 
> The other places where the ParseSession was used are adapted to
> operate on the value type instead of the pointer, where appropriate.
> 
> This patch hopefully fixes the assertions we currently trigger in LLVM
> debug builds. Further optimizations, to e.g. enable concurrent
> read-access, can be investigated later on. I'm not really sure it's
> going to be worth the effort though...
> 
> 
> Diffs
> -----
> 
>   clangparsejob.h 91fe788853991b2450964907c4649873a17cc0e9 
>   clangparsejob.cpp 50b3da507df2410e1c8876d0de52bd67cb51084c 
>   codecompletion/context.h 3e9c6358cd5638a051938d6b4a1f46c3cbdc352e 
>   codecompletion/context.cpp af79ac0350bd4e9aa1373ce894de673bd67efd65 
>   codecompletion/model.cpp 92cec373ee8d4e62966db85fe960849f19e396d1 
>   codegen/clangsignatureassistant.cpp 07357ff23b1d8056b075f6176f99c5038d41201d 
>   duchain/clanghelpers.h 2a759f078fe0baa5b1add514e8af06ad20756c56 
>   duchain/clanghelpers.cpp 3d7c7d9eabb2844671f0a531bea9f2857f0f4c08 
>   duchain/clangpch.h 7adf8fb70f39bb0c1400d649264620a3cc86a8f5 
>   duchain/clangpch.cpp 4f3687b300603a689f48343cad5396b61637a897 
>   duchain/parsesession.h 7754daba425de1cb7149761743994b818740ac8a 
>   duchain/parsesession.cpp a297e2d1f7a79511d1058d8ce06fd43a70875a62 
>   tests/clang-parser.cpp de00f654e1899e71504fdc922a9885838ce94f45 
>   tests/test_codecompletion.cpp 7c95da8c1fb6b5910d738f5eba6fd78cc5ab10e7 
>   tests/test_problems.cpp cc6dddc149eaae540f17509a70def5fa814675e5 
> 
> Diff: https://git.reviewboard.kde.org/r/118817/diff/
> 
> 
> Testing
> -------
> 
> Tests pass and manual testing on a small project seems to work fine as well. I don't have a debug build of llvm though, so if anyone could test that it would be appreciated.
> 
> 
> Thanks,
> 
> Milian Wolff
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140618/3618e942/attachment.html>