[gentoo-portage-dev] [PATCH] SpawnProcess: re-check cgroup.procs until empty (bug 566420)

[gentoo-portage-dev] [PATCH] SpawnProcess: re-check cgroup.procs until empty (bug 566420)

[Zac Medico]

For subshell die support (bug 465008), we need to re-check cgroup.procs
until it's empty, in case any of the listed processes fork before we've
had a chance to kill them.

X-Gentoo-Bug: 566420
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=566420
---
 pym/_emerge/SpawnProcess.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/pym/_emerge/SpawnProcess.py b/pym/_emerge/SpawnProcess.py
index 5ab2e67..b27be2f 100644
--- a/pym/_emerge/SpawnProcess.py
+++ b/pym/_emerge/SpawnProcess.py
@@ -203,10 +203,13 @@ class SpawnProcess(SubProcess):
 						elif e.errno != errno.ESRCH:
 							raise
 
-			# step 1: kill all orphans
-			pids = get_pids(self.cgroup)
-			if pids:
-				kill_all(pids, signal.SIGKILL)
+			# step 1: kill all orphans (loop in case of new forks)
+			while True:
+				pids = get_pids(self.cgroup)
+				if pids:
+					kill_all(pids, signal.SIGKILL)
+				else:
+					break
 
 			# step 2: remove the cgroup
 			try:
-- 
2.4.9

[gentoo-portage-dev] [PATCH v2] SpawnProcess: re-check cgroup.procs until empty (bug 566420)

[Zac Medico]

For subshell die support (bug 465008), re-check cgroup.procs until it's
empty, in case any of the listed processes fork before we've had a chance
to kill them.

X-Gentoo-Bug: 566420
X-Gentoo-Bug-URL: https://bugs.gentoo.org/show_bug.cgi?id=566420
---
[PATCH v2] limits the cleanup loop to 8 iterations, and logs an eerror
message if there are any lingering pids.

 pym/_emerge/SpawnProcess.py | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/pym/_emerge/SpawnProcess.py b/pym/_emerge/SpawnProcess.py
index 5ab2e67..4ab646d 100644
--- a/pym/_emerge/SpawnProcess.py
+++ b/pym/_emerge/SpawnProcess.py
@@ -16,6 +16,8 @@ from _emerge.SubProcess import SubProcess
 import portage
 from portage import os
 from portage.const import BASH_BINARY
+from portage.localization import _
+from portage.output import EOutput
 from portage.util import writemsg_level
 from portage.util._async.PipeLogger import PipeLogger
 
@@ -35,6 +37,10 @@ class SpawnProcess(SubProcess):
 	__slots__ = ("args",) + \
 		_spawn_kwarg_names + ("_pipe_logger", "_selinux_type",)
 
+	# Max number of attempts to kill the processes listed in cgroup.procs,
+	# given that processes may fork before they can be killed.
+	_CGROUP_CLEANUP_RETRY_MAX = 8
+
 	def _start(self):
 
 		if self.fd_pipes is None:
@@ -203,10 +209,24 @@ class SpawnProcess(SubProcess):
 						elif e.errno != errno.ESRCH:
 							raise
 
-			# step 1: kill all orphans
-			pids = get_pids(self.cgroup)
+			# step 1: kill all orphans (loop in case of new forks)
+			remaining = self._CGROUP_CLEANUP_RETRY_MAX
+			while remaining:
+				remaining -= 1
+				pids = get_pids(self.cgroup)
+				if pids:
+					kill_all(pids, signal.SIGKILL)
+				else:
+					break
+
 			if pids:
-				kill_all(pids, signal.SIGKILL)
+				msg = []
+				msg.append(
+					_("Failed to kill pid(s) in '%(cgroup)s': %(pids)s") % dict(
+					cgroup=os.path.join(self.cgroup, 'cgroup.procs'),
+					pids=' '.join(str(pid) for pid in pids)))
+
+				self._elog('eerror', msg)
 
 			# step 2: remove the cgroup
 			try:
@@ -215,3 +235,8 @@ class SpawnProcess(SubProcess):
 				# it may be removed already, or busy
 				# we can't do anything good about it
 				pass
+
+	def _elog(self, elog_funcname, lines):
+		elog_func = getattr(EOutput(), elog_funcname)
+		for line in lines:
+			elog_func(line)
-- 
2.4.9

Re: [gentoo-portage-dev] [PATCH v2] SpawnProcess: re-check cgroup.procs until empty (bug 566420)

[Alexander Berntsen]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

This looks pretty dirty. But I guess I trust you, so ACK.

- -- 
Alexander
bernalex@gentoo.org
https://secure.plaimi.net/~alexander
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCgAGBQJWUus6AAoJENQqWdRUGk8BS/sQAN3Qw7bv/lZb5PksYqKMqIFL
PwilyLidtj4XeACurnJzsGODC/I2jhxhdBkG7bfLPMmQ0xrobJWVHKK6t2SBOtDK
M/fLz/3nuN4Anyrt0jEEqhkp4kZaJOquWr7k3Rg6o9XXtETP/Y+MKqU1vh8EkgPE
Rswk3XasOLxmzT5BtEiOIOuwUknEUB3McmLZhZfSMhwKKePtZfV2TP4AFqr8qng5
Rl4ULMQrC6jbuI6BxZySyPS14+SppkLYK5NOR/eiTRmcXqJ3tRJPr392U+Pg9dav
OxZvUcteOCK8W52kCgPy46vp/tXqmBC1Euy5vRe+y2MqoGB+4kCrqI+qy2W0WGx3
3sV5viSf0bW+pE1dKFq5eVoxdJwBKiEg/PNY1ff7GqOeXC33AacR1d0UtIuOKZ9o
ZSokwp0flhn8MalIK98q4a1HEWSlCScoKLDaqCUWpA3QxqEHX0p3iaapwgHPRaz0
JtjzOjkMkTytuVZzFgM4Ce2KgRBbgKBjdGOZDkz1oqC6zQ+y19ZBc7Bc1FMY5e5r
rKEjl56z4ereVDMNSVa6eoRtDH3sWi1qD+avUm/wpelJNAO+jg0uFuVJnKzQXIw1
WkJDbzb/1982/25W3j3ebE5AgGZB+1CAs2P6SS+8kwJC9DIUfdvbWVk4GmeuhWKw
AUQsjgXPxZTryLb0T/8t
=VjYT
-----END PGP SIGNATURE-----