[PATCH 0/4] improved process management

[PATCH 0/4] improved process management

[Ed Bartosh]

Hi,

This is a set of improvements for process management in toaster script.
It includes a bit of cleanup (first 2 patches) and quite serious changes in
the way toaster script starts and stops runbuild and runserver processes:
 - usage of process groups (PGID) instead of PID
 - usage of pid namespaces http://man7.org/linux/man-pages/man7/pid_namespaces.7.html

Both changes should hopefully make toaster to stop running processes more reliably.

One side effect and a partial reason of this work:
 it should fix #7973: stopping toaster kills ALL django apps running on the machine.

Testing instructions:
 - run toaster
 - run build in Toaster UI
 - wait until you see a lot of build-related processes in 'ps x' output
 - stop toaster. expected outcome: no toaster or build-related processes in 'ps x' output

The following changes since commit 34ccad16d40ec0685f405b8d7116250fb58e323d:

  buildinfohelper: discover kernel artifacts correctly (2016-08-23 14:29:32 +0300)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib ed/toaster/kill-toaster-pid-namespace-7973
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=ed/toaster/kill-toaster-pid-namespace-7973

Ed Bartosh (4):
  toaster: don't kill toaster on start
  toaster: remove handling of .toasterui.pid
  toaster: use process group id to kill processes
  toaster: run builds in pid namespace

 bitbake/bin/toaster | 48 ++++++++++++++++--------------------------------
 1 file changed, 16 insertions(+), 32 deletions(-)

--
Regards,
Ed

[PATCH 1/4] toaster: don't kill toaster on start

[Ed Bartosh]

There is no point of trying to kill django development server
when toaster starts as 'manage.py checksocket' command is already
used in the script code to check if development server port is occupied.

Even if Toaster is listening on another port, killing previous instance
looks quite implicit and doesn't solve anything as there are other
processes that might be still running.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 bitbake/bin/toaster | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/bitbake/bin/toaster b/bitbake/bin/toaster
index e35bfce..5e01101 100755
--- a/bitbake/bin/toaster
+++ b/bitbake/bin/toaster
@@ -249,14 +249,6 @@ case $CMD in
             fi
         fi
 
-        # kill Toaster web server if it's alive
-        if [ -e $BUILDDIR/.toastermain.pid ] && kill -0 `cat $BUILDDIR/.toastermain.pid`; then
-            echo "Warning: bitbake appears to be dead, but the Toaster web server is running." 1>&2
-            echo " Something fishy is going on." 1>&2
-            echo "Cleaning up the web server to start from a clean slate."
-            webserverKillAll
-        fi
-
         # Create configuration file
         conf=${BUILDDIR}/conf/local.conf
         line='INHERIT+="toaster buildhistory"'
-- 
2.6.6

[PATCH 2/4] toaster: remove handling of .toasterui.pid

[Ed Bartosh]

This file is not created anywhere, but handled in toaster
script code.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 bitbake/bin/toaster | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/bitbake/bin/toaster b/bitbake/bin/toaster
index 5e01101..b14749a 100755
--- a/bitbake/bin/toaster
+++ b/bitbake/bin/toaster
@@ -95,10 +95,6 @@ stop_system()
     # prevent reentry
     if [ $INSTOPSYSTEM -eq 1 ]; then return; fi
     INSTOPSYSTEM=1
-    if [ -f ${BUILDDIR}/.toasterui.pid ]; then
-        kill `cat ${BUILDDIR}/.toasterui.pid` 2>/dev/null
-        rm ${BUILDDIR}/.toasterui.pid
-    fi
     webserverKillAll
     # unset exported variables
     unset TOASTER_DIR
-- 
2.6.6

[PATCH 3/4] toaster: use process group id to kill processes

[Ed Bartosh]

Used process group id(PGID) instead of PID for process management.
This should allow to kill processes and their children more
reliably than using PIDs.

[YOCTO #7973]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 bitbake/bin/toaster | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/bitbake/bin/toaster b/bitbake/bin/toaster
index b14749a..e6a5696 100755
--- a/bitbake/bin/toaster
+++ b/bitbake/bin/toaster
@@ -26,28 +26,23 @@ Usage: source toaster start|stop [webport=<address:port>] [noweb]
 
 webserverKillAll()
 {
-    local pidfile
-    for pidfile in ${BUILDDIR}/.toastermain.pid ${BUILDDIR}/.runbuilds.pid; do
-        if [ -f ${pidfile} ]; then
-            pid=`cat ${pidfile}`
-            while kill -0 $pid 2>/dev/null; do
-                kill -SIGTERM -$pid 2>/dev/null
-                sleep 1
-                # Kill processes if they are still running - may happen
-                # in interactive shells
-                ps fux | grep "python.*manage.py runserver" | awk '{print $2}' | xargs kill
-            done
-            rm  ${pidfile}
+    local pgidfile
+    for pgidfile in ${BUILDDIR}/.runserver.pgid ${BUILDDIR}/.runbuilds.pgid; do
+        if [ -f ${pgidfile} ]; then
+            kill -KILL -`cat $pgidfile` >/dev/null 2>&1
+            rm  ${pgidfile}
         fi
     done
 }
 
 webserverStartAll()
 {
+    local pgidfile
+    pgidfile=${BUILDDIR}/.runserver.pgid
     # do not start if toastermain points to a valid process
-    if ! cat "${BUILDDIR}/.toastermain.pid" 2>/dev/null | xargs -I{} kill -0 {} ; then
+    if ! cat $pgidfile 2>/dev/null | xargs -I{} kill -0 -{} ; then
         retval=1
-        rm "${BUILDDIR}/.toastermain.pid"
+        rm $pgidfile
     fi
 
     retval=0
@@ -72,13 +67,13 @@ webserverStartAll()
 
     $MANAGE runserver "$ADDR_PORT" \
            </dev/null >>${BUILDDIR}/toaster_web.log 2>&1 \
-           & echo $! >${BUILDDIR}/.toastermain.pid
+           & ps -o pgid= $! | grep -o '[0-9]\+' >$pgidfile
 
     sleep 1
 
-    if ! cat "${BUILDDIR}/.toastermain.pid" | xargs -I{} kill -0 {} ; then
+    if ! cat $pgidfile | xargs -I{} kill -0 -{} ; then
         retval=1
-        rm "${BUILDDIR}/.toastermain.pid"
+        rm $pgidfile
     else
         echo "Toaster development webserver started at http://$ADDR_PORT"
         echo -e "\nYou can now run 'bitbake <target>' on the command line and monitor your build in Toaster.\nYou can also use a Toaster project to configure and run a build.\n"
@@ -156,7 +151,7 @@ fi
 # 1) clones of layers (in _toaster_clones )
 # 2) the build dir (in build)
 # 3) the sqlite db if that is being used.
-# 4) pid's we need to clean up on exit/shutdown
+# 4) pgid's we need to clean up on exit/shutdown
 # note: for future. in order to make this an arbitrary directory, we need to
 # make sure that the toaster.sqlite file doesn't default to `pwd` like it currently does.
 export TOASTER_DIR=`pwd`
@@ -214,7 +209,7 @@ fi
 # 1) clones of layers (in _toaster_clones )
 # 2) the build dir (in build)
 # 3) the sqlite db if that is being used.
-# 4) pid's we need to clean up on exit/shutdown
+# 4) pgid's we need to clean up on exit/shutdown
 # note: for future. in order to make this an arbitrary directory, we need to
 # make sure that the toaster.sqlite file doesn't default to `pwd`
 # like it currently does.
@@ -255,7 +250,8 @@ case $CMD in
             return 4
         fi
         export BITBAKE_UI='toasterui'
-        $MANAGE runbuilds & echo $! >${BUILDDIR}/.runbuilds.pid
+        $MANAGE runbuilds &
+        ps -o pgid= $! | grep -o '[0-9]\+' > ${BUILDDIR}/.runbuilds.pgid
         # set fail safe stop system on terminal exit
         trap stop_system SIGHUP
         echo "Successful ${CMD}."
-- 
2.6.6

[PATCH 4/4] toaster: run builds in pid namespace

[Ed Bartosh]

Used pid namespace for runbuilds process. As runbuilds becomes
an init(pid 1) in its pid namespace, killing it will result in
killing all processes in the pid namespace(children of runbuilds process).
This way is much more effective than using PIDs or even PGIDs as
it doesn't leave any processes running.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 bitbake/bin/toaster | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bitbake/bin/toaster b/bitbake/bin/toaster
index e6a5696..3cae49b 100755
--- a/bitbake/bin/toaster
+++ b/bitbake/bin/toaster
@@ -250,7 +250,7 @@ case $CMD in
             return 4
         fi
         export BITBAKE_UI='toasterui'
-        $MANAGE runbuilds &
+        unshare --user --pid --fork $MANAGE runbuilds &
         ps -o pgid= $! | grep -o '[0-9]\+' > ${BUILDDIR}/.runbuilds.pgid
         # set fail safe stop system on terminal exit
         trap stop_system SIGHUP
-- 
2.6.6