Re: [gentoo-portage-dev] [PATCH] SpawnProcess: fix event loop recursion in _pipe_logger_exit (bug 613990)

[Brian Dolbec]

On Sun, 26 Mar 2017 23:54:37 -0700
Zac Medico <zmedico@g.o> wrote:

> Fix SpawnProcess._pipe_logger_exit to wait for process exit status
> asynchronously, in order to avoid event loop recursion. This is
> required for asyncio compatibility, and also protects emerge from
> exceeding the maximum recursion depth limit like in bug 402335.
>
> X-Gentoo-bug: 613990
> X-Gentoo-bug-url: https://bugs.gentoo.org/show_bug.cgi?id=613990
> ---
>  pym/_emerge/SpawnProcess.py |  3 +--
>  pym/_emerge/SubProcess.py   | 23 ++++++++++++++++++++++-
>  2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/pym/_emerge/SpawnProcess.py b/pym/_emerge/SpawnProcess.py
> index e046640..7326254 100644
> --- a/pym/_emerge/SpawnProcess.py
> +++ b/pym/_emerge/SpawnProcess.py
> @@ -170,8 +170,7 @@ class SpawnProcess(SubProcess):
>
>  	def _pipe_logger_exit(self, pipe_logger):
>  		self._pipe_logger = None
> -		self._unregister()
> -		self.wait()
> +		self._async_waitpid()
>
>  	def _waitpid_loop(self):
>  		SubProcess._waitpid_loop(self)
> diff --git a/pym/_emerge/SubProcess.py b/pym/_emerge/SubProcess.py
> index 13d9382..b81cfd5 100644
> --- a/pym/_emerge/SubProcess.py
> +++ b/pym/_emerge/SubProcess.py
> @@ -12,7 +12,7 @@ import errno
>  class SubProcess(AbstractPollTask):
>
>  	__slots__ = ("pid",) + \
> -		("_dummy_pipe_fd", "_files", "_reg_id")
> +		("_dummy_pipe_fd", "_files", "_reg_id",
> "_waitpid_id")
>  	# This is how much time we allow for waitpid to succeed after
>  	# we've sent a kill signal to our subprocess.
> @@ -101,6 +101,23 @@ class SubProcess(AbstractPollTask):
>
>  		return self.returncode
>
> +	def _async_waitpid(self):
> +		"""
> +		Wait for exit status of self.pid asynchronously, and
> then
> +		set the returncode and notify exit listeners. This is
> +		prefered over _waitpid_loop, since the synchronous
> nature
> +		of _waitpid_loop can cause event loop recursion.
> +		"""
> +		if self._waitpid_id is None:
> +			self._waitpid_id =
> self.scheduler.child_watch_add(
> +				self.pid, self._async_waitpid_cb)
> +
> +	def _async_waitpid_cb(self, pid, condition, user_data=None):
> +		if pid != self.pid:
> +			raise AssertionError("expected pid %s, got
> %s" % (self.pid, pid))
> +		self._set_returncode((pid, condition))
> +		self.wait()
> +
>  	def _waitpid_loop(self):
>  		source_id = self.scheduler.child_watch_add(
>  			self.pid, self._waitpid_cb)
> @@ -129,6 +146,10 @@ class SubProcess(AbstractPollTask):
>  			self.scheduler.source_remove(self._reg_id)
>  			self._reg_id = None
>
> +		if self._waitpid_id is not None:
> +
> self.scheduler.source_remove(self._waitpid_id)
> +			self._waitpid_id = None
> +
>  		if self._files is not None:
>  			for f in self._files.values():
>  				if isinstance(f, int):

looks fine

-- 
Brian Dolbec <dolsen>