[gentoo-portage-dev] [PATCH 4/5] emirrordist: add debug SIGUSR1 handler

[Zac Medico]

This is handy for debugging issues with SIGTERM/SIGINT
handling.
---
 bin/emirrordist | 8 
 1 file changed, 8 insertions(+)

diff --git a/bin/emirrordist b/bin/emirrordist
index 0368eee..17f99f5 100755
--- a/bin/emirrordist
+++ b/bin/emirrordist
@@ -2,6 +2,7 @@
 # Copyright 2013-2014 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
+import signal
 import sys
 
 import portage
@@ -10,4 +11,11 @@ portage._disable_legacy_globals()
 from portage._emirrordist.main import emirrordist_main
 
 if __name__ == "__main__":
+
+   def debug_signal(_signum, _frame):
+   import pdb
+   pdb.set_trace()
+
+   signal.signal(signal.SIGUSR1, debug_signal)
+
sys.exit(emirrordist_main(sys.argv[1:]))
-- 
2.10.2

[gentoo-portage-dev] [PATCH 5/5] FetchIterator: add terminate method

[Zac Medico]

Add a terminate method to FetchIterator so that it doesn't
delay termination of emirrordist via SIGINT. Otherwise it
it is possible for the __iter__ method to loop for a very
long time after SIGINT has been delivered. For example,
this could happen if there are many ebuilds with stale
cache and RESTRICT=mirror. This issue was discovered
during testing of changes to the MirrorDistTask.terminate
implementation.
---
 pym/portage/_emirrordist/FetchIterator.py  | 21 +
 pym/portage/_emirrordist/MirrorDistTask.py |  6 +-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/pym/portage/_emirrordist/FetchIterator.py 
b/pym/portage/_emirrordist/FetchIterator.py
index 16a0b04..3841979 100644
--- a/pym/portage/_emirrordist/FetchIterator.py
+++ b/pym/portage/_emirrordist/FetchIterator.py
@@ -1,6 +1,8 @@
 # Copyright 2013 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
+import threading
+
 from portage import os
 from portage.checksum import (_apply_hash_filter,
_filter_unaccelarated_hashes, _hash_filter)
@@ -13,6 +15,19 @@ class FetchIterator(object):
def __init__(self, config):
self._config = config
self._log_failure = config.log_failure
+   self._terminated = threading.Event()
+
+   def terminate(self):
+   """
+   Schedules early termination of the __iter__ method, which is
+   useful because under some conditions it's possible for __iter__
+   to loop for a long time without yielding to the caller. For
+   example, it's useful when there are many ebuilds with stale
+   cache and RESTRICT=mirror.
+
+   This method is thread-safe (and safe for signal handlers).
+   """
+   self._terminated.set()
 
def _iter_every_cp(self):
# List categories individually, in order to start yielding 
quicker,
@@ -37,6 +52,9 @@ class FetchIterator(object):
 
for cp in self._iter_every_cp():
 
+   if self._terminated.is_set():
+   return
+
for tree in portdb.porttrees:
 
# Reset state so the Manifest is pulled once
@@ -46,6 +64,9 @@ class FetchIterator(object):
 
for cpv in portdb.cp_list(cp, mytree=tree):
 
+   if self._terminated.is_set():
+   return
+
try:
restrict, = portdb.aux_get(cpv, 
("RESTRICT",),
mytree=tree)
diff --git a/pym/portage/_emirrordist/MirrorDistTask.py 
b/pym/portage/_emirrordist/MirrorDistTask.py
index 0702eb1..8da9f06 100644
--- a/pym/portage/_emirrordist/MirrorDistTask.py
+++ b/pym/portage/_emirrordist/MirrorDistTask.py
@@ -32,9 +32,11 @@ class MirrorDistTask(CompositeTask):
self._config = config
self._term_rlock = threading.RLock()
self._term_callback_handle = None
+   self._fetch_iterator = None
 
def _start(self):
-   fetch = TaskScheduler(iter(FetchIterator(self._config)),
+   self._fetch_iterator = FetchIterator(self._config)
+   fetch = TaskScheduler(iter(self._fetch_iterator),
max_jobs=self._config.options.jobs,
max_load=self._config.options.load_average,
event_loop=self._config.event_loop)
@@ -226,6 +228,8 @@ class MirrorDistTask(CompositeTask):
self._term_callback)
 
def _term_callback(self):
+   if self._fetch_iterator is not None:
+   self._fetch_iterator.terminate()
self.cancel()
self.wait()
 
-- 
2.10.2

[gentoo-portage-dev] [PATCH 2/5] PollScheduler: terminate via call_soon for asyncio compat

[Zac Medico]

Use call_soon to schedule the _termination_check callback when needed.
The previous idle_add usage was relatively inefficient, because it
scheduled the _termination_check callback to be called in every
iteration of the event loop.

Add a _cleanup method to handle cleanup of callbacks registered with
the global event loop. Since the terminate method is thread safe and it
interacts with self._term_callback_handle, use this variable only while
holding a lock.
---
 pym/_emerge/PollScheduler.py  | 57 +++
 pym/_emerge/Scheduler.py  |  7 ++--
 pym/portage/util/_async/AsyncScheduler.py | 16 -
 3 files changed, 54 insertions(+), 26 deletions(-)

diff --git a/pym/_emerge/PollScheduler.py b/pym/_emerge/PollScheduler.py
index b118ac1..569879b 100644
--- a/pym/_emerge/PollScheduler.py
+++ b/pym/_emerge/PollScheduler.py
@@ -25,8 +25,10 @@ class PollScheduler(object):
a non-main thread)
@type main: bool
"""
+   self._term_rlock = threading.RLock()
self._terminated = threading.Event()
self._terminated_tasks = False
+   self._term_check_handle = None
self._max_jobs = 1
self._max_load = None
self._scheduling = False
@@ -44,6 +46,21 @@ class PollScheduler(object):
def _is_background(self):
return self._background
 
+   def _cleanup(self):
+   """
+   Cleanup any callbacks that have been registered with the global
+   event loop.
+   """
+   # The self._term_check_handle attribute requires locking
+   # since it's modified by the thread safe terminate method.
+   with self._term_rlock:
+   if self._term_check_handle not in (None, False):
+   self._term_check_handle.cancel()
+   # This prevents the terminate method from scheduling
+   # any more callbacks (since _cleanup must eliminate all
+   # callbacks in order to ensure complete cleanup).
+   self._term_check_handle = False
+
def terminate(self):
"""
Schedules asynchronous, graceful termination of the scheduler
@@ -51,26 +68,36 @@ class PollScheduler(object):
 
This method is thread-safe (and safe for signal handlers).
"""
-   self._terminated.set()
+   with self._term_rlock:
+   if self._term_check_handle is None:
+   self._terminated.set()
+   self._term_check_handle = 
self._event_loop.call_soon_threadsafe(
+   self._termination_check, True)
 
-   def _termination_check(self):
+   def _termination_check(self, retry=False):
"""
Calls _terminate_tasks() if appropriate. It's guaranteed not to
-   call it while _schedule_tasks() is being called. The check 
should
-   be executed for each iteration of the event loop, for response 
to
-   termination signals at the earliest opportunity. It always 
returns
-   True, for continuous scheduling via idle_add.
+   call it while _schedule_tasks() is being called. This method 
must
+   only be called via the event loop thread.
+
+   @param retry: If True then reschedule if scheduling state 
prevents
+   immediate termination.
+   @type retry: bool
"""
-   if not self._scheduling and \
-   self._terminated.is_set() and \
+   if self._terminated.is_set() and \
not self._terminated_tasks:
-   self._scheduling = True
-   try:
-   self._terminated_tasks = True
-   self._terminate_tasks()
-   finally:
-   self._scheduling = False
-   return True
+   if not self._scheduling:
+   self._scheduling = True
+   try:
+   self._terminated_tasks = True
+   self._terminate_tasks()
+   finally:
+   self._scheduling = False
+
+   elif retry:
+   with self._term_rlock:
+   self._term_check_handle = 
self._event_loop.call_soon(
+   self._termination_check, True)
 
def _terminate_tasks(self):
"""
diff --git a/pym/_emerge/Scheduler.py 

Re: [gentoo-portage-dev] [PATCH] phase-helpers.sh: Loop over A rather than SRC_URI in __eapi0_pkg_nofetch.

[Ulrich Mueller]

> On Thu, 23 Mar 2017, Zac Medico wrote:

> On Thu, Mar 23, 2017 at 2:55 AM, Ulrich Müller  wrote:
>> Looping over SRC_URI also outputs non-filename elements (e.g, use
>> conditionals) which is avoided when looping over A.
>> 
>> Gentoo-Bug: 613132
>> ---
>> bin/phase-helpers.sh | 8 
>> 1 file changed, 4 insertions(+), 4 deletions(-)

> Looks good.

Presumably we should wait for the ack (or rather the usual consensus
timeout) of the corresponding PMS patch:
https://archives.gentoo.org/gentoo-pms/message/6044537cd2cd36bc23307918a9afced9

Ulrich


pgp1eMEcTz3O8.pgp
Description: PGP signature

Re: [gentoo-portage-dev] [PATCH] phase-helpers.sh: Loop over A rather than SRC_URI in __eapi0_pkg_nofetch.

[Zac Medico]

On Thu, Mar 23, 2017 at 2:55 AM, Ulrich Müller  wrote:
> Looping over SRC_URI also outputs non-filename elements (e.g, use
> conditionals) which is avoided when looping over A.
>
> Gentoo-Bug: 613132
> ---
>  bin/phase-helpers.sh | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Looks good.
-- 
Thanks,
Zac

[gentoo-portage-dev] [PATCH] phase-helpers.sh: Loop over A rather than SRC_URI in __eapi0_pkg_nofetch.

[Ulrich Müller]

Looping over SRC_URI also outputs non-filename elements (e.g, use
conditionals) which is avoided when looping over A.

Gentoo-Bug: 613132
---
 bin/phase-helpers.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
index 9e4e6a2f8..e1dcfd5e8 100644
--- a/bin/phase-helpers.sh
+++ b/bin/phase-helpers.sh
@@ -1,5 +1,5 @@
 #!/bin/bash
-# Copyright 1999-2013 Gentoo Foundation
+# Copyright 1999-2017 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
 export DESTTREE=/usr
@@ -726,11 +726,11 @@ einstall() {
 }
 
 __eapi0_pkg_nofetch() {
-   [ -z "${SRC_URI}" ] && return
+   [[ -z ${A} ]] && return
 
-   elog "The following are listed in SRC_URI for ${PN}:"
+   elog "The following files cannot be fetched for ${PN}:"
local x
-   for x in $(echo ${SRC_URI}); do
+   for x in ${A}; do
elog "   ${x}"
done
 }
-- 
2.12.1


pgp529zGwiQ94.pgp
Description: PGP signature