Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 730192 - sys-apps/portage: replace os.fork with multiprocessing.Process
Summary: sys-apps/portage: replace os.fork with multiprocessing.Process
Status: RESOLVED FIXED
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Core (show other bugs)
Hardware: All All
: Normal enhancement (vote)
Assignee: Portage team
URL:
Whiteboard:
Keywords: InVCS
Depends on:
Blocks: 733180
  Show dependency tree
 
Reported: 2020-06-30 00:06 UTC by Zac Medico
Modified: 2020-09-18 18:44 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 2020-06-30 00:06:46 UTC
There's a lot that could go wrong when using os.fork to fork a python interpreter, so in theory it should be safer to use a higher level wrapper like multiprocessing.Process to handle all of the nuances.
Comment 1 Zac Medico gentoo-dev 2020-07-03 01:52:28 UTC
Since multiprocessing.Process closes sys.stdin, a workaround will be needed if there are any forks that need to inherit stdin for things like PROPERTIES=interactive support.
Comment 2 Zac Medico gentoo-dev 2020-07-03 06:20:40 UTC
I've got a working implementation here:

https://github.com/gentoo/portage/pull/565
Comment 3 Zac Medico gentoo-dev 2020-07-16 20:31:44 UTC
(In reply to Zac Medico from comment #2)
> I've got a working implementation here:
> 
> https://github.com/gentoo/portage/pull/565

Made some tweaks and posted here for review now:

https://archives.gentoo.org/gentoo-portage-dev/message/1f60f37f177bbeb132c651bdfcc4b346
Comment 4 Larry the Git Cow gentoo-dev 2020-07-19 05:41:08 UTC
The bug has been referenced in the following commit(s):

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

commit bde44b75407dfe0a390033636894a136af4e7533
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2020-07-02 23:51:41 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2020-07-19 04:35:23 +0000

    ForkProcess: replace os.fork with multiprocessing.Process (bug 730192)
    
    Replace os.fork with multiprocessing.Process, in order to leverage
    any pre-fork and post-fork interpreter housekeeping that it provides,
    promoting a healthy state for the forked interpreter.
    
    Since multiprocessing.Process closes sys.__stdin__, fix relevant
    code to use the portage._get_stdin() compatibility function.
    In case there's a legitimate need to inherit stdin for things like
    PROPERTIES=interactive support, create a temporary duplicate of
    fd_pipes[0] when appropriate, and restore sys.stdin and sys.__stdin__
    in the subprocess.
    
    Bug: https://bugs.gentoo.org/730192
    Signed-off-by: Zac Medico <zmedico@gentoo.org>

 lib/portage/process.py                 |   4 +-
 lib/portage/sync/controller.py         |   4 +-
 lib/portage/util/_async/ForkProcess.py | 146 ++++++++++++++++++++++++++-------
 3 files changed, 119 insertions(+), 35 deletions(-)
Comment 5 Zac Medico gentoo-dev 2020-07-19 05:51:52 UTC
ForkProcess accounted for most forks. Now MergeProcess needs to be migrated to use ForkProcess._spawn, since it currently overrides it entirely.
Comment 7 Larry the Git Cow gentoo-dev 2020-07-22 17:46:54 UTC
The bug has been referenced in the following commit(s):

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

commit 3561071e07ad47db91bf0f2c2c2b02e2061b217c
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2020-07-19 06:31:37 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2020-07-22 16:55:33 +0000

    MergeProcess: replace os.fork with multiprocessing.Process (bug 730192)
    
    Fix the MergeProcess _spawn method to call the superclass _spawn
    method, in order to replace os.fork with multiprocessing.Process,
    promoting a healthy state for the forked interpreter.
    
    Bug: https://bugs.gentoo.org/730192
    Signed-off-by: Zac Medico <zmedico@gentoo.org>

 lib/portage/dbapi/_MergeProcess.py | 106 +++++++++++--------------------------
 1 file changed, 30 insertions(+), 76 deletions(-)
Comment 8 Larry the Git Cow gentoo-dev 2020-07-22 20:42:09 UTC
The bug has been referenced in the following commit(s):

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

commit c14ac733a4e05990973d99e13f19aaf9bde57bcb
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2020-07-22 20:16:42 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2020-07-22 20:20:11 +0000

    _EbuildFetcherProcess: emit eerror for fetch failure in  _proc_join_done
    
    Since ForkProcess now receives process exit status in the
    _proc_join_done method instead of the _async_waitpid_cb method,
    _EbuildFetcherProcess needs to emit eerror messages there
    instead.
    
    Fixes: bde44b75407d ("ForkProcess: replace os.fork with multiprocessing.Process (bug 730192)")
    Bug: https://bugs.gentoo.org/730192
    Signed-off-by: Zac Medico <zmedico@gentoo.org>

 lib/_emerge/EbuildFetcher.py | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
Comment 9 Larry the Git Cow gentoo-dev 2020-07-27 03:39:44 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=def7124a8e502517b721a942f568a4e24dbaf81a

commit def7124a8e502517b721a942f568a4e24dbaf81a
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2020-07-27 03:29:56 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2020-07-27 03:34:12 +0000

    sys-apps/portage: Bump to version 3.0.1
    
     #730192 Replace os.fork with multiprocessing.Process, and fix
             regression in portage-3.0.0 involving eerror messages
             for fetch failures
    
    Bug: https://bugs.gentoo.org/733180
    Bug: https://bugs.gentoo.org/730192
    Package-Manager: Portage-3.0.1, Repoman-2.3.23
    Signed-off-by: Zac Medico <zmedico@gentoo.org>

 sys-apps/portage/Manifest             |   1 +
 sys-apps/portage/portage-3.0.1.ebuild | 263 ++++++++++++++++++++++++++++++++++
 2 files changed, 264 insertions(+)