Re: [PATCH] _get_lock_fn: support multiprocessing spawn start method (bug 758230)

Fabian Groffen <[email protected]> Sat, 5 Dec 2020 11:35:06 +0100

Newsgroups	gmane.linux.gentoo.portage.devel
Organization	Gentoo Foundation, Inc.
Message-ID	<[email protected]>

Thanks Zac!

On 04-12-2020 14:58:22 -0800, Zac Medico wrote:
> Ensure that _get_lock_fn arguments to multiprocessing.Process will
> successfully pickle, as required by the spawn start method, which
> is the default for macOS since Python 3.8.
> 
> Since file descriptors are not inherited unless the fork start
> method is used, the subprocess should only try to close an
> inherited file descriptor for the fork start method.
> 
> Bug: https://bugs.gentoo.org/758230
> Signed-off-by: Zac Medico <[email protected]>
> ---
>  lib/portage/locks.py | 36 +++++++++++++++++++++++-------------
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/portage/locks.py b/lib/portage/locks.py
> index 1073343be..193045c03 100644
> --- a/lib/portage/locks.py
> +++ b/lib/portage/locks.py
> @@ -44,18 +44,7 @@ def _get_lock_fn():
>  	if _lock_fn is not None:
>  		return _lock_fn
>  
> -	def _test_lock(fd, lock_path):
> -		os.close(fd)
> -		try:
> -			with open(lock_path, 'a') as f:
> -				fcntl.lockf(f.fileno(), fcntl.LOCK_EX|fcntl.LOCK_NB)
> -		except EnvironmentError as e:
> -			if e.errno == errno.EAGAIN:
> -				# Parent process holds lock, as expected.
> -				sys.exit(0)
>  
> -		# Something went wrong.
> -		sys.exit(1)
>  
>  	fd, lock_path = tempfile.mkstemp()
>  	try:
> @@ -64,8 +53,16 @@ def _get_lock_fn():
>  		except EnvironmentError:
>  			pass
>  		else:
> -			proc = multiprocessing.Process(target=_test_lock,
> -				args=(fd, lock_path))
> +			proc = multiprocessing.Process(
> +				target=_subprocess_test_lock,
> +				args=(
> +					# Since file descriptors are not inherited unless the fork start
> +					# method is used, the subprocess should only try to close an
> +					# inherited file descriptor for the fork start method.
> +					fd if multiprocessing.get_start_method() == "fork" else None,
> +					lock_path,
> +				),
> +			)
>  			proc.start()
>  			proc.join()
>  			if proc.exitcode == os.EX_OK:
> @@ -80,6 +77,19 @@ def _get_lock_fn():
>  	_lock_fn = fcntl.flock
>  	return _lock_fn
>  
> +def _subprocess_test_lock(fd, lock_path):
> +	if fd is not None:
> +		os.close(fd)
> +	try:
> +		with open(lock_path, 'a') as f:
> +			fcntl.lockf(f.fileno(), fcntl.LOCK_EX|fcntl.LOCK_NB)
> +	except EnvironmentError as e:
> +		if e.errno == errno.EAGAIN:
> +			# Parent process holds lock, as expected.
> +			sys.exit(0)
> +
> +	# Something went wrong.
> +	sys.exit(1)
>  
>  _open_fds = {}
>  _open_inodes = {}
> -- 
> 2.26.2
> 
> 

-- 
Fabian Groffen
Gentoo on a different level

signature.asc (application/pgp-signature, 488 B)