| msg16629 - (view) |
Author: Troels Walsted Hansen (troels) |
Date: 2003-06-27 14:58 |
| 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() |
|
|
| msg16630 - (view) |
Author: Neal Norwitz (nnorwitz) *  |
Date: 2003-06-29 03:09 |
| 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). |
|
|
| msg16631 - (view) |
Author: Neal Norwitz (nnorwitz) * |
Date: 2003-06-29 03:10 |
| Logged In: YES user_id=33168 Tim, for 2.3b2 or 2.3.1? |
|
|
| msg16632 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2003-06-30 15:57 |
| 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? |
|
|
| msg16633 - (view) |
Author: Neal Norwitz (nnorwitz) * |
Date: 2003-07-01 03:40 |
| 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? |
|
|
| msg16634 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2003-07-09 16:39 |
| 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. |
|
|
| msg16635 - (view) |
Author: Michael Hudson (mwh) |
Date: 2003-07-10 16:30 |
| 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. |
|
|
| msg16636 - (view) |
Author: Andrew Gaul (gaul) |
Date: 2003-10-01 18:50 |
| Logged In: YES user_id=139865 Patch #816059 includes a less ugly fix for this bug. |
|
|