Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/ --- 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
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/#review72655 --- Unfortunataly it doesn't fix it here, plasmashell starts with a blank screen. kquitapp5 plasmashell && plasmashell brings it back - Lukáš Tinkl On Pro. 28, 2014, 11:33 dop., David Edmundson wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121710/ > --- > > (Updated Pro. 28, 2014, 11:33 dop.) > > > 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
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
> On Dec. 29, 2014, 7:25 a.m., Lukáš Tinkl wrote: > > Unfortunataly it doesn't fix it here, plasmashell starts with a blank > > screen. kquitapp5 plasmashell && plasmashell brings it back Works for me.. :S which XRandr version you have? - Bhushan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/#review72655 --- On Dec. 28, 2014, 4:03 p.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, 4:03 p.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
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
> On Dec. 29, 2014, 1:55 a.m., Lukáš Tinkl wrote: > > Unfortunataly it doesn't fix it here, plasmashell starts with a blank > > screen. kquitapp5 plasmashell && plasmashell brings it back > > Bhushan Shah wrote: > Works for me.. :S which XRandr version you have? That's depressing to hear. I need some help on this; logs, traces, something as I can't reproduce here and this bug is super duper critical. Do you agree this change still makes sense though? Even if there is another problem it will be easier if there's one potential bug less. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/#review72655 --- 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
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
--- 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
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
> On Dec. 29, 2014, 1:55 a.m., Lukáš Tinkl wrote: > > Unfortunataly it doesn't fix it here, plasmashell starts with a blank > > screen. kquitapp5 plasmashell && plasmashell brings it back > > Bhushan Shah wrote: > Works for me.. :S which XRandr version you have? > > David Edmundson wrote: > That's depressing to hear. I need some help on this; logs, traces, > something as I can't reproduce here and this bug is super duper critical. > > Do you agree this change still makes sense though? Even if there is > another problem it will be easier if there's one potential bug less. Unfortunately I seem to have hit a worse situation. kquitapp5 plasmashell does not work in my case as it just fails. Killing plasmashell and then starting it just delivers the same situation, a black screen with plasmashell running in the background and that is it. Starting plasmashell shows the following output: kscreen: launcherDataAvailable: "org.kde.KScreen.Backend.XRandR" kscreen: Launcher finished with exit code 1 , status 0 kscreen: Service for requested backend already running And yes I have a kscreen_backend_launcher process running, which is started by kded5. When I try to kill this one, kded5 is automatically restarting it. To be honest this patch makes things worse as that plasmashell is not starting at all. - Raymond --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/#review72655 --- 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
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
> On Dec. 29, 2014, 1:55 a.m., Lukáš Tinkl wrote: > > Unfortunataly it doesn't fix it here, plasmashell starts with a blank > > screen. kquitapp5 plasmashell && plasmashell brings it back > > Bhushan Shah wrote: > Works for me.. :S which XRandr version you have? > > David Edmundson wrote: > That's depressing to hear. I need some help on this; logs, traces, > something as I can't reproduce here and this bug is super duper critical. > > Do you agree this change still makes sense though? Even if there is > another problem it will be easier if there's one potential bug less. > > Raymond Wooninck wrote: > Unfortunately I seem to have hit a worse situation. kquitapp5 > plasmashell does not work in my case as it just fails. Killing plasmashell > and then starting it just delivers the same situation, a black screen with > plasmashell running in the background and that is it. Starting plasmashell > shows the following output: > kscreen: launcherDataAvailable: "org.kde.KScreen.Backend.XRandR" > kscreen: Launcher finished with exit code 1 , status 0 > kscreen: Service for requested backend already running > > And yes I have a kscreen_backend_launcher process running, which is > started by kded5. When I try to kill this one, kded5 is automatically > restarting it. > > To be honest this patch makes things worse as that plasmashell is not > starting at all. Just forgot: raymond@HQVMT44011:~% xrandr --version xrandr program version 1.4.3 Server reports RandR version 1.4 - Raymond --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/#review72655 --- 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
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
> On Dec. 29, 2014, 7:25 a.m., Lukáš Tinkl wrote: > > Unfortunataly it doesn't fix it here, plasmashell starts with a blank > > screen. kquitapp5 plasmashell && plasmashell brings it back > > Bhushan Shah wrote: > Works for me.. :S which XRandr version you have? > > David Edmundson wrote: > That's depressing to hear. I need some help on this; logs, traces, > something as I can't reproduce here and this bug is super duper critical. > > Do you agree this change still makes sense though? Even if there is > another problem it will be easier if there's one potential bug less. > > Raymond Wooninck wrote: > Unfortunately I seem to have hit a worse situation. kquitapp5 > plasmashell does not work in my case as it just fails. Killing plasmashell > and then starting it just delivers the same situation, a black screen with > plasmashell running in the background and that is it. Starting plasmashell > shows the following output: > kscreen: launcherDataAvailable: "org.kde.KScreen.Backend.XRandR" > kscreen: Launcher finished with exit code 1 , status 0 > kscreen: Service for requested backend already running > > And yes I have a kscreen_backend_launcher process running, which is > started by kded5. When I try to kill this one, kded5 is automatically > restarting it. > > To be honest this patch makes things worse as that plasmashell is not > starting at all. > > Raymond Wooninck wrote: > Just forgot: > raymond@HQVMT44011:~% xrandr --version > xrandr program version 1.4.3 > Server reports RandR version 1.4 >kscreen: launcherDataAvailable: "org.kde.KScreen.Backend.XRandR" >kscreen: Launcher finished with exit code 1 , status 0 >kscreen: Service for requested backend already running I had same output but with this patch I am not getting that.. and plasmashell works for me fine. - Bhushan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/#review72655 --- On Dec. 28, 2014, 4:03 p.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, 4:03 p.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
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
> On Dec. 29, 2014, 1:55 a.m., Lukáš Tinkl wrote: > > Unfortunataly it doesn't fix it here, plasmashell starts with a blank > > screen. kquitapp5 plasmashell && plasmashell brings it back > > Bhushan Shah wrote: > Works for me.. :S which XRandr version you have? > > David Edmundson wrote: > That's depressing to hear. I need some help on this; logs, traces, > something as I can't reproduce here and this bug is super duper critical. > > Do you agree this change still makes sense though? Even if there is > another problem it will be easier if there's one potential bug less. > > Raymond Wooninck wrote: > Unfortunately I seem to have hit a worse situation. kquitapp5 > plasmashell does not work in my case as it just fails. Killing plasmashell > and then starting it just delivers the same situation, a black screen with > plasmashell running in the background and that is it. Starting plasmashell > shows the following output: > kscreen: launcherDataAvailable: "org.kde.KScreen.Backend.XRandR" > kscreen: Launcher finished with exit code 1 , status 0 > kscreen: Service for requested backend already running > > And yes I have a kscreen_backend_launcher process running, which is > started by kded5. When I try to kill this one, kded5 is automatically > restarting it. > > To be honest this patch makes things worse as that plasmashell is not > starting at all. > > Raymond Wooninck wrote: > Just forgot: > raymond@HQVMT44011:~% xrandr --version > xrandr program version 1.4.3 > Server reports RandR version 1.4 > > Bhushan Shah wrote: > >kscreen: launcherDataAvailable: "org.kde.KScreen.Backend.XRandR" > >kscreen: Launcher finished with exit code 1 , status 0 > >kscreen: Service for requested backend already running > > I had same output but with this patch I am not getting that.. and > plasmashell works for me fine. Ok. I dived a little deeper into the issues that I got (as that reverting the patch didn't help) and it appeared that I had to clean my config files. I deleted all the "plasma*" files/directories from .config and after this I was able to start plasmashell again. A new problem arose with the kcache files for the plasma theme, which caused plasma freezes. Deleting those made my installation happy and now I can boot normally and plasmashell is starting. Lukas, you might want to give this patch a try with a clean user, to see if it works. However it seems not to resolve the issue completely as that another person indicated that with a new user, he now gets the old issue despite that with his normal user everything works fine. - Raymond --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/#review72655 --- 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 htt
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
> On Pro. 29, 2014, 2:55 dop., Lukáš Tinkl wrote: > > Unfortunataly it doesn't fix it here, plasmashell starts with a blank > > screen. kquitapp5 plasmashell && plasmashell brings it back > > Bhushan Shah wrote: > Works for me.. :S which XRandr version you have? > > David Edmundson wrote: > That's depressing to hear. I need some help on this; logs, traces, > something as I can't reproduce here and this bug is super duper critical. > > Do you agree this change still makes sense though? Even if there is > another problem it will be easier if there's one potential bug less. > > Raymond Wooninck wrote: > Unfortunately I seem to have hit a worse situation. kquitapp5 > plasmashell does not work in my case as it just fails. Killing plasmashell > and then starting it just delivers the same situation, a black screen with > plasmashell running in the background and that is it. Starting plasmashell > shows the following output: > kscreen: launcherDataAvailable: "org.kde.KScreen.Backend.XRandR" > kscreen: Launcher finished with exit code 1 , status 0 > kscreen: Service for requested backend already running > > And yes I have a kscreen_backend_launcher process running, which is > started by kded5. When I try to kill this one, kded5 is automatically > restarting it. > > To be honest this patch makes things worse as that plasmashell is not > starting at all. > > Raymond Wooninck wrote: > Just forgot: > raymond@HQVMT44011:~% xrandr --version > xrandr program version 1.4.3 > Server reports RandR version 1.4 > > Bhushan Shah wrote: > >kscreen: launcherDataAvailable: "org.kde.KScreen.Backend.XRandR" > >kscreen: Launcher finished with exit code 1 , status 0 > >kscreen: Service for requested backend already running > > I had same output but with this patch I am not getting that.. and > plasmashell works for me fine. > > Raymond Wooninck wrote: > Ok. I dived a little deeper into the issues that I got (as that reverting > the patch didn't help) and it appeared that I had to clean my config files. I > deleted all the "plasma*" files/directories from .config and after this I was > able to start plasmashell again. A new problem arose with the kcache files > for the plasma theme, which caused plasma freezes. Deleting those made my > installation happy and now I can boot normally and plasmashell is starting. > > Lukas, you might want to give this patch a try with a clean user, to see > if it works. However it seems not to resolve the issue completely as that > another person indicated that with a new user, he now gets the old issue > despite that with his normal user everything works fine. No change, even with a new user. KScreen also routinely forgets my settings and sets the external monitor as a clone (although my setup is an extension to the right side) and unsets the primary display to be empty. - Lukáš --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/#review72655 --- On Pro. 28, 2014, 11:33 dop., David Edmundson wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121710/ > --- > > (Updated Pro. 28, 2014, 11:33 dop.) > > > 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://gi
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/#review72782 --- Ship it! I guess the code is note perfect and there are still race conditions, but this is a good step in the right direction, so let's ship it. I'll think about the lockfile solution, or some other and try to implement it (once I'm back at work). The one-service-multiple-objects solution would not work, since you can't have multiple processes owning the same DBus service name, and I don't want to run multiple backends in one process. - Daniel Vrátil On Dec. 28, 2014, 11: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, 11: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
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
> On Dec. 30, 2014, 2:06 p.m., Daniel Vrátil wrote: > > I guess the code is note perfect and there are still race conditions, but > > this is a good step in the right direction, so let's ship it. I'll think > > about the lockfile solution, or some other and try to implement it (once > > I'm back at work). > > > > The one-service-multiple-objects solution would not work, since you can't > > have multiple processes owning the same DBus service name, and I don't want > > to run multiple backends in one process. Why would you ever want to run multiple backends at the same time? (outside of testing, which could be done on a private DBus session) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/#review72782 --- 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
Re: Review Request 121710: Avoid risk of starting two kscreen_launchers at the same having race conditions
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121710/ --- (Updated Jan. 2, 2015, 11:20 a.m.) Status -- This change has been marked as submitted. 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
