Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 614108 - sys-apps/portage: _LockProcess.unlock triggers event loop recursion
Summary: sys-apps/portage: _LockProcess.unlock triggers event loop recursion
Status: RESOLVED FIXED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Core - Interface (emerge) (show other bugs)
Hardware: All All
: Normal normal (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords: InVCS
Depends on:
Blocks: 591760 651804
  Show dependency tree
 
Reported: 2017-03-28 04:00 UTC by Zac Medico
Modified: 2018-07-02 18:40 UTC (History)
0 users

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Zac Medico gentoo-dev 2017-03-28 04:00:11 UTC
The _LockProcess.unlock method calls self._proc.wait() before the process exit status has become available, which will ultimately triggering recursion in the SubProcess._waitpid_loop method. This means that _LockProcess will need an async_unlock method that takes a callback argument or returns a Future.
Comment 2 Zac Medico gentoo-dev 2017-04-03 20:13:39 UTC
The asycn_unlock patch is in the master branch:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=916a0733c7201b7a8b22f5262bd5be8cbc8992a6

Now we need to to migrate the callers of the unlock method to call async_unlock instead.
Comment 3 Zac Medico gentoo-dev 2018-04-18 22:23:06 UTC
The AsyncTaskFuture class may be a handy way to adapt existing code to deal with the Future that the async_unlock method returns.
Comment 4 Zac Medico gentoo-dev 2018-04-19 07:53:19 UTC
I've posted a patch to use async_unlock EbuildPhase._ebuild_exit:

https://archives.gentoo.org/gentoo-portage-dev/message/799170765e9183dafc0b2d4ac9b6cb83
https://github.com/gentoo/portage/pull/306
Comment 5 Zac Medico gentoo-dev 2018-04-19 17:06:58 UTC
I've posted a patch to add an EbuildBuildDir async_unlock method, and a patch to make EbuildBuild use it:

https://archives.gentoo.org/gentoo-portage-dev/message/854cb77934df4be5f5bbf6920d332a16
https://github.com/gentoo/portage/pull/307
Comment 6 Larry the Git Cow gentoo-dev 2018-04-20 15:45:41 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/portage.git/commit/?id=2f1816dd7a8ba68a857ea9081431f0a5ab90ee4c

commit 2f1816dd7a8ba68a857ea9081431f0a5ab90ee4c
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2018-04-19 02:50:57 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2018-04-19 07:33:30 +0000

    EbuildPhase._ebuild_exit: use async_unlock (bug 614108)
    
    Use async_unlock to avoid event loop recursion, and AsyncTaskFuture
    to fit the resulting future into the CompositeTask framework that
    EbuildPhase uses.
    
    Bug: https://bugs.gentoo.org/614108

 pym/_emerge/EbuildPhase.py | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)}
Comment 7 Larry the Git Cow gentoo-dev 2018-04-20 16:18:24 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/portage.git/commit/?id=90fa156df0e6ef4fa9ef1a80c495511f4630de86

commit 90fa156df0e6ef4fa9ef1a80c495511f4630de86
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2018-04-20 15:21:58 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2018-04-20 15:50:43 +0000

    EbuildBuildDir: remove synchronous unlock method (bug 614108)
    
    The synchronous unlock 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_unlock method, and remove the synchronous unlock method.
    
    Bug: https://bugs.gentoo.org/614108
    Bug: https://bugs.gentoo.org/649588

 pym/_emerge/EbuildBuildDir.py               | 21 ---------------------
 pym/_emerge/Scheduler.py                    |  2 +-
 pym/portage/dbapi/vartree.py                |  3 ++-
 pym/portage/package/ebuild/doebuild.py      |  9 ++++++---
 pym/portage/tests/ebuild/test_ipc_daemon.py |  2 +-
 5 files changed, 10 insertions(+), 27 deletions(-)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=87c079175c7a504ae893ed7d6ced03638d4cc853

commit 87c079175c7a504ae893ed7d6ced03638d4cc853
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2018-04-20 05:39:31 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2018-04-20 15:50:42 +0000

    AbstractEbuildProcess: use async_unlock (bug 614108)
    
    Override the _async_wait method to asynchronously unlock
    self._build_dir when necessary, override the _wait method in order
    to function as a failsafe if _async_wait has not been called for
    some reason, and fix the SubProcess superclass to call _async_wait
    when appropriate.
    
    Execution of the _wait method's failsafe code will automatically
    become a fatal error at the same time as event loop recursion is
    disabled.
    
    Bug: https://bugs.gentoo.org/614108

 pym/_emerge/AbstractEbuildProcess.py | 65 +++++++++++++++++++++++++++++++++---
 pym/_emerge/SubProcess.py            |  2 +-
 2 files changed, 61 insertions(+), 6 deletions(-)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=720fef408d07e6aeb4ca81f39c38df3085367795

commit 720fef408d07e6aeb4ca81f39c38df3085367795
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2018-04-20 05:21:19 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2018-04-20 15:50:33 +0000

    Binpkg: use async_unlock (bug 614108)
    
    Add an _async_unlock_builddir method which accepts a
    returncode parameter for cases where it should set the
    returncode and notify exit listeners.
    
    Bug: https://bugs.gentoo.org/614108

 pym/_emerge/Binpkg.py | 50 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 17 deletions(-)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=7f7174748266197aa3112f7c6c93a255d1d18ed0

commit 7f7174748266197aa3112f7c6c93a255d1d18ed0
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2018-04-20 04:49:25 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2018-04-20 15:50:19 +0000

    PackageUninstall: use async_unlock (bug 614108)
    
    Add an _async_unlock_builddir method which accepts a
    returncode parameter for cases where it should set the
    returncode and notify exit listeners.
    
    Bug: https://bugs.gentoo.org/614108

 pym/_emerge/PackageUninstall.py | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=5ca8ef781952d8148c21a1f5369c6d04335a9208

commit 5ca8ef781952d8148c21a1f5369c6d04335a9208
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2018-04-19 16:18:36 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2018-04-20 15:49:59 +0000

    EbuildBuild: use async_unlock (bug 614108)
    
    Add an _async_unlock_builddir method which accepts a
    returncode parameter for cases where it should set the
    returncode and notify exit listeners.
    
    Bug: https://bugs.gentoo.org/614108

 pym/_emerge/EbuildBuild.py | 52 ++++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 20 deletions(-)

https://gitweb.gentoo.org/proj/portage.git/commit/?id=0783351b0cb69fc33b984b1fe51e6c642c3bab7d

commit 0783351b0cb69fc33b984b1fe51e6c642c3bab7d
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2018-04-19 16:12:00 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2018-04-20 15:49:41 +0000

    EbuildBuildDir: add async_unlock method (bug 614108)
    
    Call the existing AsynchronousLock async_unlock method
    for the build directory lock, and also handle removal of the
    category directory (with async lock/unlock).
    
    Bug: https://bugs.gentoo.org/614108

 pym/_emerge/EbuildBuildDir.py | 46 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)}
Comment 8 Zac Medico gentoo-dev 2018-04-20 16:21:05 UTC
The EbuildBuildDir.lock() method can also trigger event loop recursion, so we'll have to convert that into an EbuildBuildDir.async_lock() method.
Comment 9 Zac Medico gentoo-dev 2018-04-21 09:13:00 UTC
After the patch series from bug 614112, BinpkgFetcher (bug 614110) is the only remaining consumer of the AsynchronousLock.unlock method.
Comment 10 Larry the Git Cow gentoo-dev 2018-04-21 18:43:48 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/portage.git/commit/?id=87a21a06c8829be6ded41a4c06dcfc19f8ffefc2

commit 87a21a06c8829be6ded41a4c06dcfc19f8ffefc2
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2018-04-21 17:32:49 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2018-04-21 18:41:58 +0000

    BinpkgFetcher: use async_unlock (bug 614108)
    
    Convert BinpkgFetcher to a CompositeTask in order to handle asynchronous
    unlock, and move remaining BinpkgFetcher code to a _BinpkgFetcherProcess
    class that still inherits SpawnProcess.
    
    Bug: https://bugs.gentoo.org/614108

 pym/_emerge/BinpkgFetcher.py | 57 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 13 deletions(-)}
Comment 11 Larry the Git Cow gentoo-dev 2018-04-22 21:52:05 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/portage.git/commit/?id=a68bf18050580eaf60bc0447558c63346b985049

commit a68bf18050580eaf60bc0447558c63346b985049
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2018-04-21 08:47:07 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2018-04-22 18:30:54 +0000

    AsynchronousLock: remove synchronous unlock method (bug 614108)
    
    The synchronous unlock 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_unlock method, and remove the synchronous unlock method.
    
    Bug: https://bugs.gentoo.org/614108
    Bug: https://bugs.gentoo.org/649588

 pym/_emerge/AsynchronousLock.py                   | 32 -----------------------
 pym/portage/tests/locks/test_asynchronous_lock.py | 22 +++++-----------
 2 files changed, 7 insertions(+), 47 deletions(-)}
Comment 12 Zac Medico gentoo-dev 2018-07-02 18:40:25 UTC
Fixed in portage-2.3.40-r1.