[Twisted-Python] [PATCH] to make twisted more "child process friendly"
Hello, Working on apt-proxy v2 I got stuck trying to get the status of a subprocess: internet/default.py:76 signal.signal(signal.SIGCHLD, process.reapProcess) process.reapProcess will be called for every child that exists, which makes imposible to anyone to get the status. And actually makes useless calling 'reapProcess' from Process.maybeCallProcessEnded The patch allows objects to be registered to get the termination status of certain processes and changes process.Process to use it. I know that now the inline documentation gets outdated, I am willing to update it if you guys like the patch. Take care ranty -- --- Manuel Estrada Sainz <ranty@debian.org> <ranty@bigfoot.com> <ranty@users.sourceforge.net> ------------------------ <manuel.estrada@hispalinux.es> ------------------- God grant us the serenity to accept the things we cannot change, courage to change the things we can, and wisdom to know the difference.
On Fri, Jun 21, 2002 at 04:18:35PM +0200, Manuel Estrada Sainz wrote:
Hello,
Working on apt-proxy v2 I got stuck trying to get the status of a subprocess:
internet/default.py:76 signal.signal(signal.SIGCHLD, process.reapProcess)
process.reapProcess will be called for every child that exists, which makes imposible to anyone to get the status. And actually makes useless calling 'reapProcess' from Process.maybeCallProcessEnded
The patch allows objects to be registered to get the termination status of certain processes and changes process.Process to use it.
That patch has a problem, if you use registerReapProccessHandler but the process finished before you actually called registerReapProccessHandler you will wait for ever on a dead child. The attached patch makes two variations: - reapProcess only takes the status of registered pid's, the rest are left alone. - registerReapProccessHandler checks if the child already exited and if so directly calls the callback. This also has the side efect of being more friendly to standard python popen, which will again be able to return the status on close(). Thanks for a great job, ranty PS: Twisted rocks :) -- --- Manuel Estrada Sainz <ranty@debian.org> <ranty@bigfoot.com> <ranty@users.sourceforge.net> ------------------------ <manuel.estrada@hispalinux.es> ------------------- God grant us the serenity to accept the things we cannot change, courage to change the things we can, and wisdom to know the difference. ? diff.diff ? diff.diff2 Index: process.py =================================================================== RCS file: /cvs/Twisted/twisted/internet/process.py,v retrieving revision 1.16 diff -u -r1.16 process.py --- process.py 26 Jun 2002 06:47:50 -0000 1.16 +++ process.py 4 Jul 2002 15:17:07 -0000 @@ -40,6 +40,7 @@ import abstract, main from main import CONNECTION_LOST, CONNECTION_DONE +reapProcessHandlers = {} def reapProcess(*args): """Reap as many processes as possible (without blocking) via waitpid. @@ -52,10 +53,32 @@ UNIX has no way to be really sure that your process is going to go away w/o blocking. I don't want to block.) """ + for pid in reapProcessHandlers.keys(): + try: + pid, status = os.waitpid(pid,os.WNOHANG) + except: + pid = None + if pid: + reapProcessHandlers[pid].processEnded(status) + del reapProcessHandlers[pid] + +def registerReapProccessHandler(pid, process): + if reapProcessHandlers.has_key(pid): + raise RuntimeError try: - os.waitpid(0,os.WNOHANG) + aux_pid, status = os.waitpid(pid,os.WNOHANG) except: - pass + aux_pid = None + if aux_pid: + process.processEnded(status) + else: + reapProcessHandlers[pid] = process + +def unregisterReapProccessHandler(pid, process): + if not (reapProcessHandlers.has_key(pid) + and reapProcessHandlers[pid] == process): + raise RuntimeError + del reapProcessHandlers[pid] class ProcessWriter(abstract.FileDescriptor, styles.Ephemeral): """(Internal) Helper class to write to Process's stdin. @@ -184,8 +207,8 @@ stdout_read, stdout_write = os.pipe() stderr_read, stderr_write = os.pipe() stdin_read, stdin_write = os.pipe() - pid = os.fork() - if pid == 0: # pid is 0 in the child process + self.pid = os.fork() + if self.pid == 0: # pid is 0 in the child process # stop debugging, if I am! I don't care anymore! sys.settrace(None) # Destroy my stdin / stdout / stderr (in that order) @@ -246,6 +269,7 @@ self.proto.makeConnection(self) except: log.deferr() + registerReapProccessHandler(self.pid, self) def closeStdin(self): """Call this to close standard input on this process. @@ -301,17 +325,23 @@ lostErrorConnection = 0 lostOutConnection = 0 lostInConnection = 0 + lostProcess = 0 def maybeCallProcessEnded(self): if (self.lostErrorConnection and self.lostOutConnection and - self.lostInConnection): + self.lostInConnection and + self.lostProcess): try: self.proto.processEnded() except: log.deferr() - reapProcess() + def processEnded(self, status): + self.status = status + self.lostProcess = 1 + self.maybeCallProcessEnded() + def inConnectionLost(self): del self.writer self.lostInConnection = 1
From: Manuel Estrada Sainz <ranty-bulk@ranty.ddts.net> Subject: Re: [Twisted-Python] [PATCH] to make twisted more "child process friendly" Date: Thu, 4 Jul 2002 17:29:04 +0200
That patch has a problem, if you use registerReapProccessHandler but the process finished before you actually called registerReapProccessHandler you will wait for ever on a dead child.
Yeah, stuff like that's the reason for the shotgun approach that was in place before.
The attached patch makes two variations:
Thanks for the update! I'll look at it closely and apply it soon.
PS: Twisted rocks :)
Indeed it does :-) -- | <`'> | Glyph Lefkowitz: Traveling Sorcerer | | < _/ > | Lead Developer, the Twisted project | | < ___/ > | http://www.twistedmatrix.com |
participants (2)
-
Glyph Lefkowitz
-
Manuel Estrada Sainz