[gentoo-portage-dev] [PATCH 0/6] EbuildBuildDir: add async_lock method (bug 614112)

[Zac Medico]

Asynchronoulsy handle locking of both the category directory and
build directory.

Bug: https://bugs.gentoo.org/614112

Zac Medico (6):
EbuildBuildDir: add async_lock method (bug 614112)
AbstractEbuildProcess: use async_lock (bug 614112)
Binpkg: use async_lock (bug 614112)
EbuildBuild: use async_lock (bug 614112)
PackageUninstall: use async_lock (bug 614112)
EbuildBuildDir: remove synchronous lock method (bug 614112)

pym/_emerge/AbstractEbuildProcess.py | 30 +++++++-
pym/_emerge/Binpkg.py | 16 +++-
pym/_emerge/EbuildBuild.py | 7 +-
pym/_emerge/EbuildBuildDir.py | 115 +++++++++++++++++-----------
pym/_emerge/PackageUninstall.py | 8 +-
pym/_emerge/Scheduler.py | 2 +-
pym/portage/dbapi/vartree.py | 2 +-
pym/portage/package/ebuild/doebuild.py | 9 ++-
pym/portage/tests/ebuild/test_ipc_daemon.py | 2 +-
9 files changed, 133 insertions(+), 58 deletions(-)

[Zac Medico]

Asynchronoulsy handle locking of both the category directory and
build directory.

Bug: https://bugs.gentoo.org/614112
---
pym/_emerge/EbuildBuildDir.py | 76 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)

diff --git a/pym/_emerge/EbuildBuildDir.py b/pym/_emerge/EbuildBuildDir.py
index 69f694992..9163deacf 100644
--- a/pym/_emerge/EbuildBuildDir.py
+++ b/pym/_emerge/EbuildBuildDir.py
@@ -1,6 +1,8 @@
# Copyright 1999-2012 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2

+import functools
+
from _emerge.AsynchronousLock import AsynchronousLock

import portage
@@ -84,6 +86,80 @@ class EbuildBuildDir(SlotObject):
except OSError:
pass

+ def async_lock(self):
+ """
+ Acquire the lock asynchronously. Notification is available
+ via the add_done_callback method of the returned Future instance.
+
+ This raises an AlreadyLocked exception if async_lock() is called
+ while a lock is already held. In order to avoid this, call
+ async_unlock() or check whether the "locked" attribute is True
+ or False before calling async_lock().
+
+ @returns: Future, result is None
+ """
+ if self._lock_obj is not None:
+ raise self.AlreadyLocked((self._lock_obj,))
+
+ dir_path = self.settings.get('PORTAGE_BUILDDIR')
+ if not dir_path:
+ raise AssertionError('PORTAGE_BUILDDIR is unset')
+ catdir = os.path.dirname(dir_path)
+ self._catdir = catdir
+ catdir_lock = AsynchronousLock(path=catdir, scheduler=self.scheduler)
+ builddir_lock = AsynchronousLock(path=dir_path, scheduler=self.scheduler)
+ result = self.scheduler.create_future()
+
+ def catdir_locked(catdir_lock):
+ try:
+ self._assert_lock(catdir_lock)
+ except AssertionError as e:
+ result.set_exception(e)
+ return
+
+ try:
+ portage.util.ensure_dirs(catdir,
+ gid=portage.portage_gid,
+ mode=0o70, mask=0)
+ except PortageException as e:
+ if not os.path.isdir(catdir):
+ result.set_exception(e)
+ return
+
+ builddir_lock.addExitListener(builddir_locked)
+ builddir_lock.start()
+
+ def builddir_locked(builddir_lock):
+ try:
+ self._assert_lock(builddir_lock)
+ except AssertionError as e:
+ catdir_lock.async_unlock.add_done_callback(
+ functools.partial(catdir_unlocked, exception=e))
+ return
+
+ self._lock_obj = builddir_lock
+ self.locked = True
+ self.settings['PORTAGE_BUILDDIR_LOCKED'] = '1'
+ catdir_lock.async_unlock().add_done_callback(catdir_unlocked)
+
+ def catdir_unlocked(future, exception=None):
+ if not (exception is None and future.exception() is None):
+ result.set_exception(exception or future.exception())
+ else:
+ result.set_result(None)
+
+ try:
+ portage.util.ensure_dirs(os.path.dirname(catdir),
+ gid=portage.portage_gid,
+ mode=0o70, mask=0)
+ except PortageException:
+ if not os.path.isdir(os.path.dirname(catdir)):
+ raise
+
+ catdir_lock.addExitListener(catdir_locked)
+ catdir_lock.start()
+ return result
+
def async_unlock(self):
"""
Release the lock asynchronously. Release notification is available

[Zac Medico]

Asynchronously lock the build directory, and use AsyncTaskFuture
to fit the resulting future into the CompositeTask framework that
Binpkg uses.

Bug: https://bugs.gentoo.org/614112
---
pym/_emerge/Binpkg.py | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/pym/_emerge/Binpkg.py b/pym/_emerge/Binpkg.py
index 09a1fe680..e9c5ef568 100644
--- a/pym/_emerge/Binpkg.py
+++ b/pym/_emerge/Binpkg.py
@@ -112,15 +112,25 @@ class Binpkg(CompositeTask):
self.wait()
return

- pkg = self.pkg
- pkg_count = self.pkg_count
if not (self.opts.pretend or self.opts.fetchonly):
- self._build_dir.lock()
+ self._start_task(
+ AsyncTaskFuture(future=self._build_dir.async_lock()),
+ self._start_fetcher)
+ else:
+ self._start_fetcher()
+
+ def _start_fetcher(self, lock_task=None):
+ if lock_task is not None:
+ self._assert_current(lock_task)
+ lock_task.future.result()
# Initialize PORTAGE_LOG_FILE (clean_log won't work without it).
portage.prepare_build_dirs(self.settings["ROOT"], self.settings, 1)
# If necessary, discard old log so that we don't
# append to it.
self._build_dir.clean_log()
+
+ pkg = self.pkg
+ pkg_count = self.pkg_count
fetcher = BinpkgFetcher(background=self.background,
logfile=self.settings.get("PORTAGE_LOG_FILE"), pkg=self.pkg,
pretend=self.opts.pretend, scheduler=self.scheduler)

[Zac Medico]

Asynchronously lock the build directory. The asynchronous lock delays
creation of the pid, and it's possible for the _wait method to be called
before the pid is available. Therefore, AbstractEbuildProcess._wait()
must wait for the pid to become available before it can call the
SpawnProcess._wait() method.

Bug: https://bugs.gentoo.org/614112
---
pym/_emerge/AbstractEbuildProcess.py | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/pym/_emerge/AbstractEbuildProcess.py b/pym/_emerge/AbstractEbuildProcess.py
index 2aa0c4a35..d481e6046 100644
--- a/pym/_emerge/AbstractEbuildProcess.py
+++ b/pym/_emerge/AbstractEbuildProcess.py
@@ -25,7 +25,7 @@ class AbstractEbuildProcess(SpawnProcess):

__slots__ = ('phase', 'settings',) + \
('_build_dir', '_build_dir_unlock', '_ipc_daemon',
- '_exit_command', '_exit_timeout_id')
+ '_exit_command', '_exit_timeout_id', '_start_future')

_phases_without_builddir = ('clean', 'cleanrm', 'depend', 'help',)
_phases_interactive_whitelist = ('config',)
@@ -130,15 +130,19 @@ class AbstractEbuildProcess(SpawnProcess):
# since we're not displaying to a terminal anyway.
self.settings['NOCOLOR'] = 'true'

+ start_ipc_daemon = False
if self._enable_ipc_daemon:
self.settings.pop('PORTAGE_EBUILD_EXIT_FILE', None)
if self.phase not in self._phases_without_builddir:
+ start_ipc_daemon = True
if 'PORTAGE_BUILDDIR_LOCKED' not in self.settings:
self._build_dir = EbuildBuildDir(
scheduler=self.scheduler, settings=self.settings)
- self._build_dir.lock()
- self.settings['PORTAGE_IPC_DAEMON'] = "1"
- self._start_ipc_daemon()
+ self._start_future = self._build_dir.async_lock()
+ self._start_future.add_done_callback(
+ functools.partial(self._start_post_builddir_lock,
+ start_ipc_daemon=start_ipc_daemon))
+ return
else:
self.settings.pop('PORTAGE_IPC_DAEMON', None)
else:
@@ -159,6 +163,19 @@ class AbstractEbuildProcess(SpawnProcess):
else:
self.settings.pop('PORTAGE_EBUILD_EXIT_FILE', None)

+ self._start_post_builddir_lock(start_ipc_daemon=start_ipc_daemon)
+
+ def _start_post_builddir_lock(self, lock_future=None, start_ipc_daemon=False):
+ if lock_future is not None:
+ if lock_future is not self._start_future:
+ raise AssertionError('lock_future is not self._start_future')
+ self._start_future = None
+ lock_future.result()
+
+ if start_ipc_daemon:
+ self.settings['PORTAGE_IPC_DAEMON'] = "1"
+ self._start_ipc_daemon()
+
if self.fd_pipes is None:
self.fd_pipes = {}
null_fd = None
@@ -375,6 +392,11 @@ class AbstractEbuildProcess(SpawnProcess):
Execution of the failsafe code will automatically become a fatal
error at the same time as event loop recursion is disabled.
"""
+ # SpawnProcess._wait() requires the pid, so wait here for the
+ # pid to become available.
+ while self._start_future is not None:
+ self.scheduler.run_until_complete(self._start_future)
+
SpawnProcess._wait(self)

if self._build_dir is not None:

[Zac Medico]

Asynchronously lock the build directory, and use AsyncTaskFuture
to fit the resulting future into the CompositeTask framework that
EbuildBuild uses.

Bug: https://bugs.gentoo.org/614112
---
pym/_emerge/EbuildBuild.py | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/pym/_emerge/EbuildBuild.py b/pym/_emerge/EbuildBuild.py
index f1a2103a3..9d4afd0ea 100644
--- a/pym/_emerge/EbuildBuild.py
+++ b/pym/_emerge/EbuildBuild.py
@@ -150,8 +150,13 @@ class EbuildBuild(CompositeTask):

self._build_dir = EbuildBuildDir(
scheduler=self.scheduler, settings=settings)
- self._build_dir.lock()
+ self._start_task(
+ AsyncTaskFuture(future=self._build_dir.async_lock()),
+ self._start_pre_clean)

+ def _start_pre_clean(self, lock_task):
+ self._assert_current(lock_task)
+ lock_task.future.result()
# Cleaning needs to happen before fetch, since the build dir
# is used for log handling.
msg = " === (%s of %s) Cleaning (%s::%s)" % \

[Zac Medico]

Asynchronously lock the build directory, and use AsyncTaskFuture
to fit the resulting future into the CompositeTask framework that
PackageUninstall uses.

Bug: https://bugs.gentoo.org/614112
---
pym/_emerge/PackageUninstall.py | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/pym/_emerge/PackageUninstall.py b/pym/_emerge/PackageUninstall.py
index 8f19e38e4..3fe1fb0a6 100644
--- a/pym/_emerge/PackageUninstall.py
+++ b/pym/_emerge/PackageUninstall.py
@@ -55,8 +55,13 @@ class PackageUninstall(CompositeTask):

self._builddir_lock = EbuildBuildDir(
scheduler=self.scheduler, settings=self.settings)
- self._builddir_lock.lock()
+ self._start_task(
+ AsyncTaskFuture(future=self._builddir_lock.async_lock()),
+ self._start_unmerge)

+ def _start_unmerge(self, lock_task):
+ self._assert_current(lock_task)
+ lock_task.future.result()
portage.prepare_build_dirs(
settings=self.settings, cleanup=True)

@@ -74,6 +79,7 @@ class PackageUninstall(CompositeTask):
noiselevel=-1)
self._emergelog("=== Unmerging... (%s)" % (self.pkg.cpv,))

+ cat, pf = portage.catsplit(self.pkg.cpv)
unmerge_task = MergeProcess(
mycat=cat, mypkg=pf, settings=self.settings,
treetype="vartree", vartree=self.pkg.root_config.trees["vartree"],

[Zac Medico]

The synchronous lock method can trigger event loop recursion if the
event loop is already running, which is incompatible with asyncio's
default event loop. Therefore, migrate the last consumers to use the
async_lock method, and remove the synchronous lock method.

Bug: https://bugs.gentoo.org/614112
Bug: https://bugs.gentoo.org/649588
---
pym/_emerge/EbuildBuildDir.py | 47 -----------------------------
pym/_emerge/Scheduler.py | 2 +-
pym/portage/dbapi/vartree.py | 2 +-
pym/portage/package/ebuild/doebuild.py | 9 ++++--
pym/portage/tests/ebuild/test_ipc_daemon.py | 2 +-
5 files changed, 9 insertions(+), 53 deletions(-)

diff --git a/pym/_emerge/EbuildBuildDir.py b/pym/_emerge/EbuildBuildDir.py
index 9163deacf..477113db8 100644
--- a/pym/_emerge/EbuildBuildDir.py
+++ b/pym/_emerge/EbuildBuildDir.py
@@ -19,53 +19,6 @@ class EbuildBuildDir(SlotObject):
SlotObject.__init__(self, **kwargs)
self.locked = False

- def lock(self):
- """
- This raises an AlreadyLocked exception if lock() is called
- while a lock is already held. In order to avoid this, call
- unlock() or check whether the "locked" attribute is True
- or False before calling lock().
- """
- if self._lock_obj is not None:
- raise self.AlreadyLocked((self._lock_obj,))
-
- dir_path = self.settings.get('PORTAGE_BUILDDIR')
- if not dir_path:
- raise AssertionError('PORTAGE_BUILDDIR is unset')
- catdir = os.path.dirname(dir_path)
- self._catdir = catdir
-
- try:
- portage.util.ensure_dirs(os.path.dirname(catdir),
- gid=portage.portage_gid,
- mode=0o70, mask=0)
- except PortageException:
- if not os.path.isdir(os.path.dirname(catdir)):
- raise
- catdir_lock = AsynchronousLock(path=catdir, scheduler=self.scheduler)
- catdir_lock.start()
- catdir_lock.wait()
- self._assert_lock(catdir_lock)
-
- try:
- try:
- portage.util.ensure_dirs(catdir,
- gid=portage.portage_gid,
- mode=0o70, mask=0)
- except PortageException:
- if not os.path.isdir(catdir):
- raise
- builddir_lock = AsynchronousLock(path=dir_path,
- scheduler=self.scheduler)
- builddir_lock.start()
- builddir_lock.wait()
- self._assert_lock(builddir_lock)
- self._lock_obj = builddir_lock
- self.settings['PORTAGE_BUILDDIR_LOCKED'] = '1'
- finally:
- self.locked = self._lock_obj is not None
- catdir_lock.unlock()
-
def _assert_lock(self, async_lock):
if async_lock.returncode != os.EX_OK:
# TODO: create a better way to propagate this error to the caller
diff --git a/pym/_emerge/Scheduler.py b/pym/_emerge/Scheduler.py
index a248f5974..71fe3e07d 100644
--- a/pym/_emerge/Scheduler.py
+++ b/pym/_emerge/Scheduler.py
@@ -799,7 +799,7 @@ class Scheduler(PollScheduler):
settings["PORTAGE_BUILDDIR"] = build_dir_path
build_dir = EbuildBuildDir(scheduler=sched_iface,
settings=settings)
- build_dir.lock()
+ sched_iface.run_until_complete(build_dir.async_lock())
current_task = None

try:
diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
index 8ad6957a3..1a86940f1 100644
--- a/pym/portage/dbapi/vartree.py
+++ b/pym/portage/dbapi/vartree.py
@@ -2081,7 +2081,7 @@ class dblink(object):
builddir_lock = EbuildBuildDir(
scheduler=scheduler,
settings=self.settings)
- builddir_lock.lock()
+ scheduler.run_until_complete(builddir_lock.async_lock())
prepare_build_dirs(settings=self.settings, cleanup=True)
log_path = self.settings.get("PORTAGE_LOG_FILE")

diff --git a/pym/portage/package/ebuild/doebuild.py b/pym/portage/package/ebuild/doebuild.py
index bdcdfbe87..3c8414387 100644
--- a/pym/portage/package/ebuild/doebuild.py
+++ b/pym/portage/package/ebuild/doebuild.py
@@ -813,7 +813,8 @@ def doebuild(myebuild, mydo, _unused=DeprecationWarning, settings=None, debug=0,
scheduler=(portage._internal_caller and
global_event_loop() or EventLoop(main=False)),
settings=mysettings)
- builddir_lock.lock()
+ builddir_lock.scheduler.run_until_complete(
+ builddir_lock.async_lock())
try:
return _spawn_phase(mydo, mysettings,
fd_pipes=fd_pipes, returnpid=returnpid)
@@ -939,7 +940,8 @@ def doebuild(myebuild, mydo, _unused=DeprecationWarning, settings=None, debug=0,
scheduler=(portage._internal_caller and
global_event_loop() or EventLoop(main=False)),
settings=mysettings)
- builddir_lock.lock()
+ builddir_lock.scheduler.run_until_complete(
+ builddir_lock.async_lock())
try:
_spawn_phase("clean", mysettings)
finally:
@@ -963,7 +965,8 @@ def doebuild(myebuild, mydo, _unused=DeprecationWarning, settings=None, debug=0,
scheduler=(portage._internal_caller and
global_event_loop() or EventLoop(main=False)),
settings=mysettings)
- builddir_lock.lock()
+ builddir_lock.scheduler.run_until_complete(
+ builddir_lock.async_lock())
mystatus = prepare_build_dirs(myroot, mysettings, cleanup)
if mystatus:
return mystatus
diff --git a/pym/portage/tests/ebuild/test_ipc_daemon.py b/pym/portage/tests/ebuild/test_ipc_daemon.py
index b45177f7e..bc18cdf64 100644
--- a/pym/portage/tests/ebuild/test_ipc_daemon.py
+++ b/pym/portage/tests/ebuild/test_ipc_daemon.py
@@ -60,7 +60,7 @@ class IpcDaemonTestCase(TestCase):
build_dir = EbuildBuildDir(
scheduler=event_loop,
settings=env)
- build_dir.lock()
+ event_loop.run_until_complete(build_dir.async_lock())
ensure_dirs(env['PORTAGE_BUILDDIR'])

input_fifo = os.path.join(env['PORTAGE_BUILDDIR'], '.ipc_in')