Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions

[Aleix Pol Gonzalez]

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


I like how the code ends up being simpler. I am still not convinced there's no space for race conditions here. Maybe we want to use something like QLockFile?
http://doc.qt.io/qt-5/qlockfile.html

- Aleix Pol Gonzalez


On Dec. 28, 2014, 10:33 a.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121710/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2014, 10:33 a.m.)
> 
> 
> Review request for Plasma, Solid and Daniel Vrátil.
> 
> 
> Repository: libkscreen
> 
> 
> Description
> -------
> 
> Avoid risk of starting two kscreen_launchers at the same having a race condition.
> 
> There were three possible bugs:
> CheckIsAlreadyRunning tried to register a service and check if it worked.
> This could clash with another process checking at the same time. Causing them both to fail saying another is running
> Similarly, a daemon doing actual registering could clash with another daemon just checking if the name is free, and then it would fail saying we can't init()
>  
> There was also a risk that two launchers pass the check that nothing is running, then both try to activate a session. DBus server handles this fine and one will gracefully fail. Without this patch the second launcher would just die without returning the path of the service that was activated causing the relevant app to do nothing.
> 
> 
> --
> 
> IMHO, you'd be better off having a fixed service name and using DBus activation for exactly these reasons.
> You could put the different backends at different object paths, and have a method on the root object that says which object path to use rather than using the stdout of a launcher. That's a discussion for another day though.
> 
> 
> Diffs
> -----
> 
>   src/backendlauncher/backendloader.cpp e7da8cd 
>   src/backendlauncher/main.cpp f8bf323 
> 
> Diff: https://git.reviewboard.kde.org/r/121710/diff/
> 
> 
> Testing
> -------
> 
> Send it to bshah. Plasmashell started for him. Previously it didn't.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20141229/34294759/attachment-0001.html>