Gentoo Archives: gentoo-portage-dev

From: Jason Stubbs <jstubbs@g.o>
To: gentoo-portage-dev@l.g.o
Subject: Re: [gentoo-portage-dev] [5/7] portage_exec cleanups
Date: Sat, 29 Oct 2005 15:25:20
Message-Id: 200510300025.23180.jstubbs@gentoo.org
In Reply to: Re: [gentoo-portage-dev] [5/7] portage_exec cleanups by Jason Stubbs
1 On Saturday 29 October 2005 19:56, Jason Stubbs wrote:
2 > All file descriptors opened by spawn() are now closed in the correct
3 > places. The code that closes all unnecessary descriptors via brute force is
4 > no longer required.
5 >
6 > I found that a file descriptor was being left open and passed around.
7 > However, it was definately not created in spawn(). It seems to me though
8 > that code outside of spawn should take care of closing (or telling spawn to
9 > close) any unnecessary FDs.
10
11
12 On Saturday 29 October 2005 23:22, Brian Harring wrote:
13 > fd_unused doesn't cut it either also, or at the very least introduces
14 > an ass biter hidden behaviour into spawn- consider
15 >
16 > fd_pipes={}
17 >
18 > Previous code *would* close all fd's, and execute the process as such.
19 > With fd_unused addition, it won't anymore- meaning at the very least a
20 > chunk of the regen code will need updating (closes stdin last I
21 > looked). Very least, all callers of spawn with fd_pipes != {0:0, 1:1,
22 > 2:2} need verification, since the handles they were having explicitly
23 > closed prior, are now left open.
24
25 Not explicitly closed. Implicitly closed.
26
27 > With the fd_unused route, there is no way clean way to have all fd's
28 > closed. You don't know what fd's are open when you spawn (nor is
29 > there any way to know, beyond testing each potential fd, or overriding
30 > the file builtin); going the fd_unused route means fd's the process
31 > shouldn't have access to, will have access to.
32
33 This is a difference of viewpoint, I think. Having unknown fds left open by
34 the calling code seems unclean to me. Iterating through 100s or 1000s of fds
35 and attempting to close them just in case the caller was sloppy seems equally
36 unclean.
37
38 By the way, this method does not allow having a pipe open to a child process
39 other than stdin, stdout or stderr.
40
41 > Mind you the fd_unused comments are about this patch and the one +2
42 > down the set.
43 >
44 > So... regressions, both in fd_pipes handling, and in open handles
45 > leaking to the spawned processes
46
47 fd_pipes handling is now addressed. The open handles leaking is an issue, but
48 where the issue stems from is yet to be resolved.
49
50 > The original code is ugly, and knowing a bit more this time around I'd
51 > write it a bit differently, that said the protections/behaviour
52 > originally in it should be left- it's more robust, and is expected
53 > now.
54
55 I disagree here. Presently, other than the execve call, every try/except block
56 in portage_exec.py has bugs that are hidden by the "protections".
57
58 --
59 Jason Stubbs
60 --
61 gentoo-portage-dev@g.o mailing list

Replies

Subject Author
Re: [gentoo-portage-dev] [5/7] portage_exec cleanups Brian Harring <ferringb@g.o>