D9016: remove global statics

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  better, but not fully correct yet.
  
  And yes, global statics are a PITA and should be avoided like hell. One easy way to achieve that is to use lazy initialization in a function-local static, since that is guaranteed to be thread safe by the compiler

INLINE COMMENTS

> helpers.cpp:318
>  {
> -    if ( Helper::documentationFileContext ) {
> -        return ReferencedTopDUContext(Helper::documentationFileContext.data());
> +    QMutexLocker lock(&cacheMutex());
> +    if ( Helper::documentationFileContext() ) {

why is this locking the cache mutex? shouldn't / couldn't you simply lock the duchain here instead?

> helpers.cpp:368
>  {
> -    if ( Helper::correctionFileDirs.isEmpty() ) {
> -        Helper::correctionFileDirs = QStandardPaths::locateAll(QStandardPaths::GenericDataLocation,
> -                                                               "kdevpythonsupport/correction_files/",
> -                                                               QStandardPaths::LocateDirectory);
> +    static QStringList correctionFileDirs;
> +    if ( correctionFileDirs.isEmpty() ) {

instead of using the mutex for this, initialize this static directly (that is guaranteed to be threadsafe with c++11):

  static const  auto correctionFileDirs = QStandardPaths::locateAll(...);

> helpers.cpp:396
> +    static QString localCorrectionFileDir;
> +    if ( localCorrectionFileDir.isEmpty() ) {
> +        QMutexLocker lock(&cacheMutex());

dito

> helpers.cpp:482
>  
>      auto project = ICore::self()->projectController()->findProjectForUrl(workingOnDocument);
>      {

this is still not thread safe due to calls like this. you should probably ensure this is only being called from the main thread. once you have done that, afaik you can remove the cacheMutex as it won't be needed anymore

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

To: brauch, flherne, mwolff
Cc: mwolff, anthonyfieroni, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171130/d25b85d8/attachment-0001.html>