On Wed, May 23, 2007 at 08:38:15PM +0100, Roger Leigh wrote:
> I hope to get it in the next release. I've been using it for a few
> months, and now that etch is released it can go in.
Excellent! :)
> The one problem I would like to get fixed first, however, is the delay
> while killing processes, if there are no processes to kill. This
> makes the typical case of running schroot or ending a session wait for
> a good few seconds. I would like it to be ~instantaneous in the case
> no processes need killing.
This is already true. If no proc items are found to kill, the function
immediately exits.
> I think the while loop needs some optimisation, such as checking if
> the process has terminated so that it can break out of the loop early.
> I think simply repeating the readlink inside the loop would be
> sufficient. So instead of "while kill", so "while readlink", and then
> kill inside the loop.
I was trading off between a 100% CPU spin on readlink vs 1 second
granularity for each process that fails to immediately die. As it is
written, if the process dies immediately, the "kill -0" will fail and no
waiting will be done.
> Another nice addition would be to print out which processes are being
> killed if in verbose mode.
Is this valuable? By the time the user sees the warning, the process is
already dead. Perhaps both the pid and the exe name?
Attached is an improved patch.
--
Kees Cook @outflux.net
--- 10mount 2007-05-23 13:23:21.000000000 -0700
+++ /etc/schroot/setup.d/10mount 2007-05-23 13:21:03.919053694 -0700
@@ -23,10 +23,49 @@
mount $VERBOSE $1 "$2" "$3"
}
+# Kill all processes that were run from within the chroot environment
+# $1: mount base location
+do_kill_all()
+{
+ if [ "$AUTH_VERBOSITY" = "verbose" ]; then
+ echo "Killing processes run inside $1"
+ fi
+ ls /proc | egrep '^[[:digit:]]+$' |
+ while read pid; do
+ exe=$(readlink /proc/"$pid"/exe || true)
+ if echo "$exe" | grep -q ^"$1"/ ; then
+ if [ "$AUTH_VERBOSITY" = "verbose" ]; then
+ echo "Killing left-over pid $pid (${exe##$1})"
+ fi
+ kill "$pid" 2>/dev/null
+
+ count=0
+ max=5
+ while [ -d /proc/"$pid"/ ] ; do
+ count=$(( $count + 1 ))
+ if [ "$AUTH_VERBOSITY" = "verbose" ]; then
+ echo "Waiting for pid $pid to shut down ... $count/$max"
+ fi
+ sleep 1
+ # Wait for $max seconds for process to die before -9'ing it
+ if [ "$count" -eq "$max" ]; then
+ if [ "$AUTH_VERBOSITY" = "verbose" ]; then
+ echo "Sending SIGKILL to pid $pid"
+ fi
+ kill -9 "$pid" 2>/dev/null
+ sleep 1
+ break
+ fi
+ done
+ fi
+ done
+}
+
# Unmount all filesystem under specified location
# $1: mount base location
do_umount_all()
{
+ do_kill_all "$1"
mounts="$("$LIBEXEC_DIR/schroot-listmounts" -m "$1")"
echo "$mounts" |
while read mountloc; do
