----------------------------------------------------------- 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 > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
