D11590: Add mutex round static member used in a thread

2018-03-24 Thread David Edmundson 
This revision was automatically updated to reflect the committed changes.
Closed by commit R120:52ceb2454be4: Add mutex round static member used in a 
thread (authored by davidedmundson).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11590?vs=30298=30455

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

AFFECTED FILES
  wallpapers/image/backgroundlistmodel.cpp
  wallpapers/image/backgroundlistmodel.h

To: davidedmundson, #plasma, mart
Cc: anthonyfieroni, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11590: Add mutex round static member used in a thread

2018-03-23 Thread David Edmundson 
davidedmundson updated this revision to Diff 30298.
davidedmundson added a comment.


  shallow-copy QStringList outside mutex

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11590?vs=30264=30298

BRANCH
  master

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

AFFECTED FILES
  wallpapers/image/backgroundlistmodel.cpp
  wallpapers/image/backgroundlistmodel.h

To: davidedmundson, #plasma
Cc: anthonyfieroni, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11590: Add mutex round static member used in a thread

2018-03-23 Thread David Edmundson 
davidedmundson added a comment.


  Very well spotted!
  
  In this specific case it's probably fine as it's never modified after 
construction, but I'll change anyway.

REPOSITORY
  R120 Plasma Workspace

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

To: davidedmundson, #plasma
Cc: anthonyfieroni, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11590: Add mutex round static member used in a thread

2018-03-22 Thread Anthony Fieroni 
anthonyfieroni added inline comments.

INLINE COMMENTS

> backgroundlistmodel.cpp:532
>  
> -m_suffixes = suffixes.toList();
> +s_suffixes = suffixes.toList();
>  }

Potentially it still can produce a crash, the function returns a reference to 
list, simultaneously read/write on list is race condition. So function can 
return a copy to static list for this function, then global variables or 
mutexes doesn't needed.

REPOSITORY
  R120 Plasma Workspace

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

To: davidedmundson, #plasma
Cc: anthonyfieroni, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D11590: Add mutex round static member used in a thread

2018-03-22 Thread David Edmundson 
davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  BackgroundFinder runs in a separate thread, it uses a static QStringList cache
  It's perfectly plausible that two BackgroundFinders threads could run at once

TEST PLAN
  Old code didn't crash, it was a pure hypothetical.
  Works as before

REPOSITORY
  R120 Plasma Workspace

BRANCH
  davidedmundson/wallpaperrewrite

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

AFFECTED FILES
  wallpapers/image/backgroundlistmodel.cpp
  wallpapers/image/backgroundlistmodel.h

To: davidedmundson, #plasma
Cc: plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart