[Python-bugs-list] [ python-Bugs-761888 ] popen2.Popen3 and popen2.Popen4 leaks filedescriptors

[SourceForge.net]

Bugs item #761888, was opened at 2003-06-27 09:58
Message generated for change (Comment added) made by gaul
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=761888&group_id=5470

Category: Python Library
Group: Python 2.3
Status: Open
Resolution: None
Priority: 5
Submitted By: Troels Walsted Hansen (troels)
Assigned to: Neal Norwitz (nnorwitz)
Summary: popen2.Popen3 and popen2.Popen4 leaks filedescriptors

Initial Comment:
The code below (from Lib/popen2.py) appears to leak no

less then 4 filedescriptors if os.fork() raises an

exception (say "OSError: [Errno 12] Not enough space"

on a Solaris box running out of swap).



Popen3.__init__() appears to leak 6 filedescriptors.



class Popen4(Popen3):

    def __init__(self, cmd, bufsize=-1):

        p2cread, p2cwrite = os.pipe()

        c2pread, c2pwrite = os.pipe()

        self.pid = os.fork()



----------------------------------------------------------------------

Comment By: Andrew Gaul (gaul)
Date: 2003-10-01 13:50

Message:
Logged In: YES 
user_id=139865

Patch #816059 includes a less ugly fix for this bug.

----------------------------------------------------------------------

Comment By: Michael Hudson (mwh)
Date: 2003-07-10 11:30

Message:
Logged In: YES 
user_id=6656

"new patch 3" looks ok, ish.



wouldn't the last try block be better written as



try:

 os.fork()

execpt OSError:

 cleanup

 raise

else:

 ...



?  It's possible I'm missing something, of course.

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2003-07-09 11:39

Message:
Logged In: YES 
user_id=6380

The fd-as-object idea has dangers too:

there may be situations where you might be

passing a fd to some extension code and never

use it again yourself -- but losing the fd would

close it! Not a good thing.  (The simples

example of this would be os.close() itself. :-)



I don't think I can review the patch adequately;

as it is quite complex I recommend that you check

it in and the point python-dev and c.l.py.ann to it.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-06-30 22:40

Message:
Logged In: YES 
user_id=33168

I tried to make it easier to review the patches by providing

2.  They were wrong though.  It was still possible to leak

file descriptors.  I believe the new patch 3 should not

allow any file descriptors to leak.



This solution is really ugly, but I can't come up with

anything cleaner.  I tried having a list of the fds that

need to be closed, but it really isn't any better.



Would it be possible (in 2.4) to make an fd type which

derives from int, so that the fd can be closed on deallocation?

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2003-06-30 10:57

Message:
Logged In: YES 
user_id=6380

I haven't reviewed the code or the fix, but as a bugfix this

could go into 2.3.



The two patch files are confusing -- why two?

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-06-28 22:10

Message:
Logged In: YES 
user_id=33168

Tim, for 2.3b2 or 2.3.1?

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2003-06-28 22:09

Message:
Logged In: YES 
user_id=33168

The attached patch should fix the problem, I'd appreciate if

you could test this.  There are 2 files attached, one is a

context diff with whitespace modifications, the other is a

minimal (no whitespace differences) patch to show what's

changed (ie, the try/except).

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=761888&group_id=5470