Add support for the DisplayStopCommand entry

[tseliot]

The DisplayStopCommand entry allows executing a script right after stopping X.

This is relevant to the following feature request:
#393

[davidedmundson]

leaks.

[tseliot]

On 02-04-15 08:55:15, davidedmundson wrote:

In [1]src/daemon/XorgDisplayServer.cpp:

@@ -233,6 +233,23 @@ namespace SDDM {
// log message
qDebug() << "Display server stopped.";

•    QString displayStopCommand = mainConfig.XDisplay.DisplayStopCommand.g
  

  et();
  +

•    // create display setup script process
  

•    QProcess *displayStopScript = new QProcess();
  

leaks.

Then we have two leaks, as XorgDisplayServer::setupDisplay() does the
same.

Alberto Milone

[davidedmundson]

should we block for it to finish?

[tseliot]

On 02-04-15 08:56:57, davidedmundson wrote:

In [1]src/daemon/XorgDisplayServer.cpp:

•    QString displayStopCommand = mainConfig.XDisplay.DisplayStopCommand.g
  

  et();
  +

•    // create display setup script process
  

•    QProcess *displayStopScript = new QProcess();
  

•    // set process environment
  

•    QProcessEnvironment env;
  

•    env.insert("DISPLAY", m_display);
  

•    env.insert("HOME", "/");
  

•    env.insert("PATH", mainConfig.Users.DefaultPath.get());
  

•    env.insert("SHELL", "/bin/sh");
  

•    displayStopScript->setProcessEnvironment(env);
  

•    // start display setup script
  

•    qDebug() << "Running display stop script " << displayStopCommand;
  

•    displayStopScript->start(displayStopCommand);
  

should we block for it to finish?

Yes, that would be a good idea. It's been a while since I last used Qt
and QProcess.

Alberto Milone

[tseliot]

How about something like this?

From 067beb73065383dd557643e39d7fc859f713a418 Mon Sep 17 00:00:00 2001
From: Alberto Milone <alberto.milone@canonical.com>
Date: Thu, 2 Apr 2015 18:24:18 +0200
Subject: [PATCH 1/1] src/daemon/XorgDisplayServer.cpp: wait for
 DisplayStopCommand to finish

Also get rid of two memory leaks
---
 src/daemon/XorgDisplayServer.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/daemon/XorgDisplayServer.cpp b/src/daemon/XorgDisplayServer.cpp
index 4f75be9..8fbfc1f 100644
--- a/src/daemon/XorgDisplayServer.cpp
+++ b/src/daemon/XorgDisplayServer.cpp
@@ -250,6 +250,14 @@ namespace SDDM {
         qDebug() << "Running display stop script " << displayStopCommand;
         displayStopScript->start(displayStopCommand);

+        // wait for finished
+        if (!displayStopScript->waitForFinished(5000))
+            displayStopScript->kill();
+
+        // clean up the script process
+        displayStopScript->deleteLater();
+        displayStopScript = nullptr;
+
         // clean up
         process->deleteLater();
         process = nullptr;
@@ -279,6 +287,10 @@ namespace SDDM {
         // start display setup script
         qDebug() << "Running display setup script " << displayCommand;
         displayScript->start(displayCommand);
+
+        // clean up the script process
+        displayScript->deleteLater();
+        displayScript = nullptr;
     }

     void XorgDisplayServer::changeOwner(const QString &fileName) {
-- 
1.9.1

[davidedmundson]

that won't work because then you're deleting the displayScript startup script potentially before it's finished.

in this case it kinda leaked for a reason... though a terrible one.

you'd need something like
connect(process, finished, process, deleteLater()));

or use QProcess::startDetatched which does it all internally.

[davidedmundson]

Thanks for looking into this.

[tseliot]

I got the code to work exactly the way I wanted in the last commit. I connected the signal as you suggested in order to free displayScript. Furthermore we now wait for displayStopScript to finish. As a result, I can easily switch between GPUs in Ubuntu now.

[tseliot]

Thanks a lot!