From 2b0345165e5af57ca61a4000c3671bbe6d677cf9 Mon Sep 17 00:00:00 2001
From: William Hubbs <w.d.hubbs@gmail.com>
Date: Thu, 14 Sep 2017 10:55:06 -0500
Subject: [PATCH] Make cgroup_cleanup send only one sigterm and sigkill

Instead of looping and sending multiple signals to child processes in
cgroup_cleanup, we send sigterm followed by sleeping one second then
sigkill.

This brings us more in line with systemd's "control group" killmode
setting.

Also, this commit includes several shellcheck cleanups.
---
 sh/openrc-run.sh.in |  6 ++--
 sh/rc-cgroup.sh.in  | 72 +++++++++++++++++++++++++--------------------
 2 files changed, 43 insertions(+), 35 deletions(-)

diff --git a/sh/openrc-run.sh.in b/sh/openrc-run.sh.in
index e778bd09..05cb972b 100644
--- a/sh/openrc-run.sh.in
+++ b/sh/openrc-run.sh.in
@@ -365,9 +365,9 @@ while [ -n "$1" ]; do
 				then
 					"$1"_post || exit $?
 				fi
-				[ "$(command -v cgroup_cleanup)" = "cgroup_cleanup" -a \
-				"$1" = "stop" ] && \
-				yesno "${rc_cgroup_cleanup}" && \
+				[ "$(command -v cgroup_cleanup)" = "cgroup_cleanup" ] &&
+					[ "$1" = "stop" ] &&
+					yesno "${rc_cgroup_cleanup}" && \
 					cgroup_cleanup
 				if [ "$(command -v cgroup2_remove)" = "cgroup2_remove" ]; then
 					[ "$1" = stop ] || [ -z "${command}" ] &&
diff --git a/sh/rc-cgroup.sh.in b/sh/rc-cgroup.sh.in
index 1bf819e7..47a007b6 100644
--- a/sh/rc-cgroup.sh.in
+++ b/sh/rc-cgroup.sh.in
@@ -14,46 +14,56 @@ description_cgroup_cleanup="Kill all processes in the cgroup"
 
 cgroup_find_path()
 {
-	local OIFS n name dir result
+	local OIFS name dir result
 	[ -n "$1" ] || return 0
 	OIFS="$IFS"
 	IFS=":"
-	while read n name dir; do
+	while read -r _ name dir; do
 		[ "$name" = "$1" ] && result="$dir"
 	done < /proc/1/cgroup
 	IFS="$OIFS"
-	echo $result
+	printf "%s" "${result}"
 }
 
 cgroup_get_pids()
 {
-	local p
-	pids=
-	while read p; do
-		[ $p -eq $$ ] || pids="${pids} ${p}"
-	done < /sys/fs/cgroup/openrc/${RC_SVCNAME}/tasks
-	[ -n "$pids" ]
+	local cgroup_procs p pids
+	cgroup_procs="$(cgroup2_find_path)"
+	[ -n "${cgroup_procs}" ] &&
+		cgroup_procs="${cgroup_procs}/${RC_SVCNAME}/cgroup.procs" ||
+		cgroup_procs="/sys/fs/cgroup/openrc/${RC_SVCNAME}/tasks"
+	[ -f "${cgroup_procs}" ] || return 0
+	while read -r p; do
+		[ "$p" -eq $$ ] || pids="${pids} ${p}"
+	done < "${cgroup_procs}"
+	printf "%s" "${pids}"
+	return 0
 }
 
 cgroup_running()
 {
-	[ -d "/sys/fs/cgroup/openrc/${RC_SVCNAME}" ]
+	[ -d "/sys/fs/cgroup/unified/${RC_SVCNAME}" ] ||
+			[ -d "/sys/fs/cgroup/${RC_SVCNAME}" ] ||
+			[ -d "/sys/fs/cgroup/openrc/${RC_SVCNAME}" ]
 }
 
 cgroup_set_values()
 {
-	[ -n "$1" -a -n "$2" -a -d "/sys/fs/cgroup/$1" ] || return 0
+	[ -n "$1" ] && [ -n "$2" ] && [ -d "/sys/fs/cgroup/$1" ] || return 0
 
-	local controller="$1" h=$(cgroup_find_path "$1")
+	local controller h
+	controller="$1"
+	h=$(cgroup_find_path "$1")
 	cgroup="/sys/fs/cgroup/${1}${h}openrc_${RC_SVCNAME}"
 	[ -d "$cgroup" ] || mkdir -p "$cgroup"
 
 	set -- $2
 	local name val
-	while [ -n "$1" -a "$controller" != "cpuacct" ]; do
+	while [ -n "$1" ] && [ "$controller" != "cpuacct" ]; do
 		case "$1" in
 			$controller.*)
-				if [ -n "$name" -a -w "$cgroup/$name" -a -n "$val" ]; then
+				if [ -n "${name}" ] && [ -w "${cgroup}/${name}" ] && 
+					[ -n "${val}" ]; then
 					veinfo "$RC_SVCNAME: Setting $cgroup/$name to $val"
 					printf "%s" "$val" > "$cgroup/$name"
 				fi
@@ -68,7 +78,7 @@ cgroup_set_values()
 		esac
 		shift
 	done
-	if [ -n "$name" -a -w "$cgroup/$name" -a -n "$val" ]; then
+	if [ -n "${name}" ] && [ -w "${cgroup}/${name}" ] && [ -n "${val}" ]; then
 		veinfo "$RC_SVCNAME: Setting $cgroup/$name to $val"
 		printf "%s" "$val" > "$cgroup/$name"
 	fi
@@ -145,7 +155,8 @@ cgroup2_find_path()
 
 cgroup2_remove()
 {
-	local cgroup_path="$(cgroup2_find_path)" rc_cgroup_path
+	local cgroup_path rc_cgroup_path
+	cgroup_path="$(cgroup2_find_path)"
 	[ -z "${cgroup_path}" ] && return 0
 	rc_cgroup_path="${cgroup_path}/${RC_SVCNAME}"
 	[ ! -d "${rc_cgroup_path}" ] ||
@@ -154,7 +165,7 @@ cgroup2_remove()
 	grep -qx "$$" "${rc_cgroup_path}/cgroup.procs" &&
 		echo 0 > "${cgroup_path}/cgroup.procs"
 	local key populated vvalue
-	while read key value; do
+	while read -r key value; do
 		case "${key}" in
 			populated) populated=${value} ;;
 			*) ;;
@@ -167,7 +178,8 @@ cgroup2_remove()
 
 cgroup2_set_limits()
 {
-	local cgroup_path="$(cgroup2_find_path)"
+	local cgroup_path
+	cgroup_path="$(cgroup2_find_path)"
 	[ -z "${cgroup_path}" ] && return 0
 	rc_cgroup_path="${cgroup_path}/${RC_SVCNAME}"
 	local OIFS="$IFS"
@@ -175,7 +187,7 @@ cgroup2_set_limits()
 "
 	[ ! -d "${rc_cgroup_path}" ] && mkdir "${rc_cgroup_path}"
 	echo 0 > "${rc_cgroup_path}/cgroup.procs"
-	echo "${rc_cgroup_settings}" | while IFS="$OIFS" read key value; do
+	echo "${rc_cgroup_settings}" | while IFS="$OIFS" read -r key value; do
 		[ -z "${key}" ] || [ -z "${value}" ] && continue
 		[ ! -e "${rc_cgroup_path}/${key}" ] && continue
 		veinfo "${RC_SVCNAME}: cgroups: ${key} ${value}"
@@ -189,17 +201,13 @@ cgroup_cleanup()
 {
 	cgroup_running || return 0
 	ebegin "starting cgroups cleanup"
-	for sig in TERM QUIT INT; do
-		cgroup_get_pids || { eend 0 "finished" ; return 0 ; }
-		for i in 0 1; do
-			kill -s $sig $pids
-			for j in 0 1 2; do
-				cgroup_get_pids || { eend 0 "finished" ; return 0 ; }
-				sleep 1
-			done
-		done 2>/dev/null
-	done
-	cgroup_get_pids || { eend 0 "finished" ; return 0; }
-	kill -9 $pids
-	eend $(cgroup_running && echo 1 || echo 0) "fail to stop all processes"
+	local pids
+	pids="$(cgroup_get_pids)"
+	if [ -n "${pids}" ]; then
+		kill -s TERM "${pids}"
+		sleep 1
+		pids="$(cgroup_get_pids)"
+		[ -n "${pids}" ] &&
+			kill -s KILL "${pids}"
+	fi
 }