Re: [gentoo-portage-dev] [PATCH] AbstractEbuildProcess: use mkdtemp to avoid cgroup interference (bug 554108)

[Brian Dolbec]

On Tue,  7 Jul 2015 00:23:45 -0700
Zac Medico <zmedico@g.o> wrote:

> This fixes parallel builds of the same package, so they don't try
> to kill eachothers processes.
>
> Fixes: b01a1b90d8c5 ("Add FEATURES=cgroup to isolate phase
> processes.") X-Gentoo-Bug: 554108
> X-Gentoo-Bug-url: https://bugs.gentoo.org/show_bug.cgi?id=554108
> ---
>  pym/_emerge/AbstractEbuildProcess.py | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/pym/_emerge/AbstractEbuildProcess.py
> b/pym/_emerge/AbstractEbuildProcess.py index 31127f4..68d96e4 100644
> --- a/pym/_emerge/AbstractEbuildProcess.py
> +++ b/pym/_emerge/AbstractEbuildProcess.py
> @@ -1,10 +1,12 @@
>  # Copyright 1999-2012 Gentoo Foundation
>  # Distributed under the terms of the GNU General Public License v2
>
> +import errno
>  import io
>  import platform
>  import stat
>  import subprocess
> +import tempfile
>  import textwrap
>  from _emerge.SpawnProcess import SpawnProcess
>  from _emerge.EbuildBuildDir import EbuildBuildDir
> @@ -14,7 +16,7 @@ from portage.elog import messages as elog_messages
>  from portage.localization import _
>  from portage.package.ebuild._ipc.ExitCommand import ExitCommand
>  from portage.package.ebuild._ipc.QueryCommand import QueryCommand
> -from portage import os
> +from portage import shutil, os
>  from portage.util._pty import _create_pty_or_pipe
>  from portage.util import apply_secpass_permissions
>
> @@ -69,9 +71,7 @@ class AbstractEbuildProcess(SpawnProcess):
>  				and self.phase not in
> self._phases_without_cgroup): cgroup_root = '/sys/fs/cgroup'
>  			cgroup_portage = os.path.join(cgroup_root,
> 'portage')
> -			cgroup_path = os.path.join(cgroup_portage,
> -					'%s:%s' %
> (self.settings["CATEGORY"],
> -						self.settings["PF"]))
> +
>  			try:
>  				# cgroup tmpfs
>  				if not os.path.ismount(cgroup_root):
> @@ -90,9 +90,9 @@ class AbstractEbuildProcess(SpawnProcess):
>  						'-o',
> 'rw,nosuid,nodev,noexec,none,name=portage', 'tmpfs', cgroup_portage])
>
> -				# the ebuild cgroup
> -				if not os.path.isdir(cgroup_path):
> -					os.mkdir(cgroup_path)
> +				cgroup_path =
> tempfile.mkdtemp(dir=cgroup_portage,
> +					prefix='%s:%s.' %
> (self.settings["CATEGORY"],
> +					self.settings["PF"]))
>  			except (subprocess.CalledProcessError,
> OSError): pass
>  			else:
> @@ -313,6 +313,13 @@ class AbstractEbuildProcess(SpawnProcess):
>  	def _set_returncode(self, wait_retval):
>  		SpawnProcess._set_returncode(self, wait_retval)
>
> +		if self.cgroup is not None:
> +			try:
> +				shutil.rmtree(self.cgroup)
> +			except EnvironmentError as e:
> +				if e.errno != errno.ENOENT:
> +					raise
> +
>  		if self._exit_timeout_id is not None:
>  			self.scheduler.source_remove(self._exit_timeout_id)
>  			self._exit_timeout_id = None

Looks good, merge away :)

-- 
Brian Dolbec <dolsen>