Re: [gentoo-portage-dev] [PATCH] Eventloop: fix deadlock involving idle_add/call_soon (bug 617550)

[Brian Dolbec]

On Fri,  5 May 2017 02:12:53 -0700
Zac Medico <zmedico@g.o> wrote:

> Guarantee that newly added idle_add/call_soon callbacks have an
> opportunity to execute before the event loop decides to wait on
> self._thread_condition without a timeout. This fixes a case where
> the event loop would wait on self._thread_condition indefinitely,
> even though a callback scheduled by the AsynchronousTask._async_wait
> method needed to be executed first.
>
> X-Gentoo-bug: 617550
> X-Gentoo-bug-url: https://bugs.gentoo.org/show_bug.cgi?id=617550
> ---
>  pym/portage/util/_eventloop/EventLoop.py | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/pym/portage/util/_eventloop/EventLoop.py
> b/pym/portage/util/_eventloop/EventLoop.py index 712838e..b7c8762
> 100644 --- a/pym/portage/util/_eventloop/EventLoop.py
> +++ b/pym/portage/util/_eventloop/EventLoop.py
> @@ -108,6 +108,15 @@ class EventLoop(object):
>  		self._poll_event_handler_ids = {}
>  		# Increment id for each new handler.
>  		self._event_handler_id = 0
> +		# New call_soon callbacks must have an opportunity to
> +		# execute before it's safe to wait on
> self._thread_condition
> +		# without a timeout, since delaying its execution
> indefinitely
> +		# could lead to a deadlock. The following attribute
> stores the
> +		# event handler id of the most recently added
> call_soon callback.
> +		# If this attribute has changed since the last time
> that the
> +		# call_soon callbacks have been called, then it's
> not safe to
> +		# wait on self._thread_condition without a timeout.
> +		self._call_soon_id = 0
>  		# Use OrderedDict in order to emulate the FIFO queue
> behavior # of the AbstractEventLoop.call_soon method.
>  		self._idle_callbacks = OrderedDict()
> @@ -250,10 +259,15 @@ class EventLoop(object):
>
>  		if not event_handlers:
>  			with self._thread_condition:
> +				prev_call_soon_id =
> self._call_soon_id if self._run_timeouts():
>  					events_handled += 1
>  				timeouts_checked = True
> -				if not event_handlers and not
> events_handled and may_block: +
> +				call_soon =
> bool(prev_call_soon_id != self._call_soon_id) +
> +				if (not call_soon and not
> event_handlers
> +					and not events_handled and
> may_block): # Block so that we don't waste cpu time by looping too
>  					# quickly. This makes
> EventLoop useful for code that needs # to wait for timeout callbacks
> regardless of whether or @@ -457,7 +471,7 @@ class EventLoop(object):
>  		@return: an integer ID
>  		"""
>  		with self._thread_condition:
> -			source_id = self._new_source_id()
> +			source_id = self._call_soon_id =
> self._new_source_id() self._idle_callbacks[source_id] =
> self._idle_callback_class( args=args, callback=callback,
> source_id=source_id) self._thread_condition.notify()


looks good
-- 
Brian Dolbec <dolsen>