Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 465008 - [Future EAPI] Mandate 'die' calls in subshells
Summary: [Future EAPI] Mandate 'die' calls in subshells
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Hosted Projects
Classification: Unclassified
Component: PMS/EAPI (show other bugs)
Hardware: All Linux
: Normal enhancement (vote)
Assignee: PMS/EAPI
URL:
Whiteboard: in-eapi-7
Keywords:
Depends on:
Blocks: future-eapi
  Show dependency tree
 
Reported: 2013-04-07 20:45 UTC by Michał Górny
Modified: 2018-04-30 22:46 UTC (History)
6 users (show)

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


Attachments
test ebuild (foo-2.ebuild,309 bytes, text/plain)
2013-06-16 08:25 UTC, Ulrich Müller
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-04-07 20:45:01 UTC
Currently PMS states:

die is not guaranteed to work correctly if called from a subshell environment.


I would like EAPI 6 to introduce a guarantee that the build process will be interrupted at the end of the phase function, in the worst case. Though it shouldn't be really hard to make it kill the main phase too.
Comment 1 Zac Medico gentoo-dev 2013-04-08 01:06:10 UTC
Since EAPI 4, helpers like emake have internal die support, which implies that the package manager has some kind of ipc system to make it happen. So, it may be feasible to apply a 'die in subshells' rule like this retroactively since EAPI 4.
Comment 2 Ulrich Müller gentoo-dev 2013-06-16 08:25:11 UTC
Created attachment 351086 [details]
test ebuild

(In reply to Zac Medico from comment #1)
> Since EAPI 4, helpers like emake have internal die support, which implies
> that the package manager has some kind of ipc system to make it happen.

The current Portage implementation is as follows:

    # subshell die support
    [[ $BASHPID = $EBUILD_MASTER_PID ]] || kill -s SIGTERM EBUILD_MASTER_PID
    exit 1

This doesn't kill all subprocesses. See attached ebuild that will hang after the die.

> So, it may be feasible to apply a 'die in subshells' rule like this
> retroactively since EAPI 4.

Let's go for EAPI 6, and we need an implementation that works reliably also in non-trivial cases.
Comment 3 Ciaran McCreesh 2013-06-16 08:27:45 UTC
(In reply to Ulrich Müller from comment #2)
> Let's go for EAPI 6, and we need an implementation that works reliably also
> in non-trivial cases.

Can we do that without patching bash?
Comment 4 Ulrich Müller gentoo-dev 2013-06-16 09:50:36 UTC
(In reply to Ciaran McCreesh from comment #3)
> (In reply to Ulrich Müller from comment #2)
> > Let's go for EAPI 6, and we need an implementation that works reliably also
> > in non-trivial cases.
> 
> Can we do that without patching bash?

I don't know. Eselect has the same problem, and there were attempts to solve it, which never worked reliably:
<http://git.overlays.gentoo.org/gitweb/?p=proj/eselect.git;a=blobdiff;f=libs/core.bash.in;h=b32e3441e4b0094f542e55afe7e3a23d7726ba04;hp=d9cda51ddea18b20cd7838418ffa334537e7f0ea;hb=4767e37ea33cabc74754d6273c4d6a9245b7075f;hpb=f37f57356ae467be4255ed1d43fdfd7599e112f7>
Comment 5 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2013-06-16 16:21:34 UTC
I think we should consider 'arbitrary subshells' and 'special subshells' (like those in multiprocessing.eclass) separately. The latter can have any additional code applied to handle the problems properly.

The logical way of handling failures in 'special subshells' would be from the failing process upwards -- that is, X dies and sends SIGTERM to its parent. The parent has a dedicated SIGTERM handler and kills its parent, and so on. If multiple children go into play, the SIGTERM handler would need to also kill the remaining children and hopefully, all processes would end up dead.

I don't know if we can portably and properly handle arbitrary subshells. I don't know if we want/need to. If there's no good solution for that, I'd rather focus on establishing a good way to handle the above case.

The universal solution on Linux would be to use cgroups. But that's not portable, and should only be introduced as a way to ensure all children end up dead even when people do wrong things. Plus a warning that children were left in cgroup.
Comment 6 Ciaran McCreesh 2013-06-16 19:28:00 UTC
This sounds to me like we're trying to do things bash wasn't designed for. Perhaps we should just encourage people to write simpler ebuilds, and stop all the unnecessary cleverness.
Comment 7 Zac Medico gentoo-dev 2013-06-16 20:44:35 UTC
(In reply to Michał Górny from comment #5)
> The universal solution on Linux would be to use cgroups. But that's not
> portable, and should only be introduced as a way to ensure all children end
> up dead even when people do wrong things. Plus a warning that children were
> left in cgroup.

The portable alternative (known as "termios") would be to create a new login session using setsid, and use the TIOCSCTTY ioctl to set a pty device as the controlling terminal. This would be analogous to what sshd does, enabling it to kill all processes from a session when a client disconnects. Programs like gnu screen and tmux must also do something similar. Package managers should be able to emulate it.
Comment 8 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2015-11-21 13:43:56 UTC
In any case, we really need this if we want to keep ebuilds sane. For example, with this never-respected rule you can't do things like:

  econf --with-foo="$(get_foo)" --with-bar="${bar}"

if get_foo can fail in any way. In fact, we would have to make get_foo fully nonfatal, and do stuff like:

  local foo bar
  foo=$(get_foo) || die
  bar=$(get_bar) || die
  econf --with-foo="${foo}" --with-bar="${bar}"

Now please tell me, how many developers are going to get *that* right?
Comment 9 Zac Medico gentoo-dev 2015-11-21 18:48:43 UTC
(In reply to Michał Górny from comment #5)
> The logical way of handling failures in 'special subshells' would be from
> the failing process upwards -- that is, X dies and sends SIGTERM to its
> parent.

In portage we could implement this using the existing ipc support in die, together with cgroup support. We just need to fix it to call the cgroup cleanup code when the die ipc call is received.
Comment 10 Larry the Git Cow gentoo-dev 2018-04-30 22:14:31 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/pms.git/commit/?id=dc0ac87f3af7271e4d9ffe48782b175159601c1e

commit dc0ac87f3af7271e4d9ffe48782b175159601c1e
Author:     Michał Górny <mgorny@gentoo.org>
AuthorDate: 2017-09-29 14:33:22 +0000
Commit:     Ulrich Müller <ulm@gentoo.org>
CommitDate: 2018-03-31 15:30:43 +0000

    EAPI 7 allows die in subshell.
    
    Bug: https://bugs.gentoo.org/465008

 eapi-differences.tex |  4 ++++
 pkg-mgr-commands.tex | 25 ++++++++++++++++---------
 2 files changed, 20 insertions(+), 9 deletions(-)}