On Sunday 30 October 2005 02:32, Jason Stubbs wrote:
> On Saturday 29 October 2005 19:34, Jason Stubbs wrote:
> > On Sunday 23 October 2005 15:45, Jason Stubbs wrote:
> > > Commented on the bug due to reasoning behind this patch. Essentially,
> > > SIGTERM is sent to "tee", a WNOHANG waitpid() is performed followed by
> > > SIGKILL if it hasn't exited. So if tee doesn't exit immediately upon
> > > getting the SIGTERM, its buffers won't get a chance to get to disk due
> > > to it being killed. This patch simply adds a 1 second window between
> > > the SIGTERM and the SIGKILL.
> >
> > Per discussion on the bug, the problem is actually that tee shouldn't be
> > sent a SIGTERM or SIGKILL at all. The decided solution was to wait on tee
> > rather ebuild. I went a bit further than that though and cleaned up most
> > of spawn().
>
> Here's the full patch after incorporating feedback. It should be fairly
> easy to follow given the other explanations. The only change beyond what's
> already been gone over is that spawn() is now removing pids from
> spawned_pids as it cleans them up.
02:30.. Brain is starting to stop working. Here's a patch of _just_
portage_exec.py. With regard to splitting up patches, I agree it's much
easier to parse a complete patch when familiar with the code. When one isn't
familiar with the code however... Splitting it up a bit makes explaining the
whys easier too - as well as keeping discussion threads seperate.
--
Jason Stubbs
Index: pym/portage_exec.py
===================================================================
--- pym/portage_exec.py (revision 2199)
+++ pym/portage_exec.py (working copy)
@@ -11,11 +11,8 @@
try:
import resource
- max_fd_limit=resource.getrlimit(RLIMIT_NOFILE)
-except SystemExit, e:
- raise
-except:
- # hokay, no resource module.
+ max_fd_limit=resource.getrlimit(resource.RLIMIT_NOFILE)[1]
+except ImportError:
max_fd_limit=256
spawned_pids = []
@@ -23,12 +20,9 @@
global spawned_pids
while spawned_pids:
pid = spawned_pids.pop()
- try:
- os.kill(pid,SIGKILL)
- except SystemExit, e:
- raise
- except:
- pass
+ if os.waitpid(pid,os.WNOHANG) == (0,0):
+ os.kill(pid,signal.SIGKILL)
+ os.waitpid(pid,0)
atexit.register(cleanup)
from portage_const import BASH_BINARY,SANDBOX_BINARY,SANDBOX_PIDS_FILE
@@ -67,6 +61,7 @@
# base spawn function
def spawn(mycommand,env={},opt_name=None,fd_pipes=None,returnpid=False,uid=None,gid=None,groups=None,umask=None,logfile=None,path_lookup=True):
+ global spawned_pids
if type(mycommand)==types.StringType:
mycommand=mycommand.split()
myc = mycommand[0]
@@ -76,21 +71,15 @@
myc = find_binary(myc)
if myc == None:
return None
-
+
+ if fd_pipes is None:
+ fd_pipes = {0:0, 1:1, 2:2}
+
mypid=[]
if logfile:
pr,pw=os.pipe()
- mypid.extend(spawn(('tee','-i','-a',logfile),returnpid=True,fd_pipes={0:pr,1:1,2:2}))
- retval=os.waitpid(mypid[-1],os.WNOHANG)[1]
- if retval != 0:
- # he's dead jim.
- if (retval & 0xff)==0:
- return (retval >> 8) # exit code
- else:
- return ((retval & 0xff) << 8) # signal
- if not fd_pipes:
- fd_pipes={}
- fd_pipes[0] = 0
+ mypid.extend(spawn(('tee','-i','-a',logfile),returnpid=True,fd_pipes={0:pr,1:fd_pipes[1],2:fd_pipes[2]}))
+ os.close(pr)
fd_pipes[1]=pw
fd_pipes[2]=pw
@@ -100,41 +89,16 @@
myargs.extend(mycommand[1:])
mypid.append(os.fork())
if mypid[-1] == 0:
- # this may look ugly, but basically it moves file descriptors around to ensure no
- # handles that are needed are accidentally closed during the final dup2 calls.
- trg_fd=[]
- if type(fd_pipes)==types.DictType:
- src_fd=[]
- k=fd_pipes.keys()
- k.sort()
- for x in k:
- trg_fd.append(x)
- src_fd.append(fd_pipes[x])
- for x in range(0,len(trg_fd)):
- if trg_fd[x] == src_fd[x]:
- continue
- if trg_fd[x] in src_fd[x+1:]:
- new=os.dup2(trg_fd[x],max(src_fd) + 1)
- os.close(trg_fd[x])
- try:
- while True:
- src_fd[s.index(trg_fd[x])]=new
- except SystemExit, e:
- raise
- except:
- pass
- for x in range(0,len(trg_fd)):
- if trg_fd[x] != src_fd[x]:
- os.dup2(src_fd[x], trg_fd[x])
- else:
- trg_fd=[0,1,2]
- for x in range(0,max_fd_limit):
- if x not in trg_fd:
- try:
+ my_pipes = {}
+ for x in fd_pipes:
+ my_pipes[x] = os.dup(fd_pipes[x])
+ for x in my_pipes:
+ os.dup2(my_pipes[x], x)
+ for x in range(max_fd_limit):
+ if x not in my_pipes:
+ try:
os.close(x)
- except SystemExit, e:
- raise
- except:
+ except OSError:
pass
# note this order must be preserved- can't change gid/groups if you change uid first.
if gid:
@@ -145,50 +109,29 @@
os.setuid(uid)
if umask:
os.umask(umask)
- try:
- # XXX: We would do this to stop ebuild.sh from getting any
- # XXX: output, and consequently, we'd get to handle the sigINT.
- #os.close(sys.stdin.fileno())
- pass
- except SystemExit, e:
- raise
- except:
- pass
try:
- #print "execing", myc, myargs
os.execve(myc,myargs,env)
- except SystemExit, e:
- raise
except Exception, e:
- raise str(e)+":\n "+myc+" "+string.join(myargs)
- # If the execve fails, we need to report it, and exit
- # *carefully* --- report error here
- os._exit(1)
- sys.exit(1)
- return # should never get reached
+ print str(e)+":\n "+myc+" "+string.join(myargs)
+ os._exit(1)
if logfile:
- os.close(pr)
os.close(pw)
if returnpid:
- global spawned_pids
spawned_pids.append(mypid[-1])
return mypid
while len(mypid):
- retval=os.waitpid(mypid[-1],0)[1]
+ pid = mypid.pop(0)
+ retval=os.waitpid(pid,0)[1]
+ if pid in spawned_pids:
+ spawned_pids.remove(pid)
if retval != 0:
- for x in mypid[0:-1]:
- try:
- os.kill(x,signal.SIGTERM)
- if os.waitpid(x,os.WNOHANG)[1] == 0:
- # feisty bugger, still alive.
- os.kill(x,signal.SIGKILL)
+ for x in mypid:
+ if os.waitpid(x,os.WNOHANG) == (0,0):
+ os.kill(x,signal.SIGKILL)
os.waitpid(x,0)
- except OSError, oe:
- if oe.errno not in (10,3):
- raise oe
# at this point we've killed all other kid pids generated via this call.
# return now.
@@ -197,8 +140,6 @@
return (retval >> 8) # return exit code
else:
return ((retval & 0xff) << 8) # interrupted by signal
- else:
- mypid.pop(-1)
return 0
def find_binary(myc):
@@ -211,5 +152,3 @@
return "%s/%s" % (x,myc)
return None
-
-
