classification
Title: FD leak in SocketServer when request handler throws exception
Type: behavior Stage: test needed
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: works for me
Dependencies: Superseder:
Assigned To: Nosy List: draghuram, giampaolo.rodola, luke-jr, martin.panter, mcjeff
Priority: high Keywords:

Created on 2007-11-12 12:27 by luke-jr, last changed 2016-02-19 22:48 by martin.panter. This issue is now closed.

Messages (9)
msg57396 - (view) Author: Luke-Jr (luke-jr) Date: 2007-11-12 12:27
SocketServer.ThreadingUnixStreamServer leaks file descriptors when a 
request handler throws an exception.
msg57471 - (view) Author: Raghuram Devarakonda (draghuram) (Python triager) Date: 2007-11-13 20:45
Please provide a small test code that manifests the problem. It is
always the best way to get quick response.
msg104814 - (view) Author: Jeff McNeil (mcjeff) * Date: 2010-05-03 04:17
I was toying with adding Unix Socket support for one of our internal tools and I thought I ran into a leak in my own code. Searched the bug tracker and found this.

I tried to reproduce, but wasn't able to. Though, if you look at the ThreadingMixIn class, you'll see this:

self.handle_error(request, client_address)
self.close_request(request)

An exception in handle_error, most likely from a subclass, would cause close_request to never fire. Though, the socket.accept'd channel would probably be shut down implicitly when leaving _handle_request_nonblock.
msg116786 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-09-18 14:37
This will go nowhere unless a patch is provided that contains code, doc and unit test changes.
msg116788 - (view) Author: Jeff McNeil (mcjeff) * Date: 2010-09-18 14:51
I'll see if I can get it to reproduce and put a patch together.
msg130788 - (view) Author: Jeff McNeil (mcjeff) * Date: 2011-03-14 04:50
I entirely forgot I had signed up to look, my apologies. 

I'm going through this w/ what's lying on Mercurial's tip, I can't reproduce it at all. I can raise exceptions of various flavors from within the handle method of a StreamRequestHandler and there are no leaking file descriptors.  

The only thing worthy of discusion, IMO, is the fact that raising an exception in a handle_error method of a subclass of BaseServer *does* cause the the self.shutdown_request to not run.  

Unless I'm mistaken, that does then leave the cleanup of that open socket to GC (but, at that point, anyone overriding handle_error method should know that). Does it make sense to run shutdown_request, even if handle_error throws an Exception?  

If anyone thinks that's worthwhile, I can do that.
msg155585 - (view) Author: Jeff McNeil (mcjeff) * Date: 2012-03-13 09:14
In an effort to walk through bugs in my nosy list, I dug into this and tried to reproduce it to no avail. 

Also, as the handle_error method is supposed to handle problems gracefully, calling shutdown on handle_error exception is probably questionable. I'd be happy to submit a patch to do just that if those smarter than I think it is worthwhile, but I don't so much believe it is.
msg235599 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-02-09 10:48
I think calling shutdown_request(), or at least close_request(), should be done regardless of whether handle_error() fails or not.
msg260533 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-19 22:48
Jeff has tried many times over two years to produce the originally reported bug without success. By code inspection, I cannot see how a request handler exception could cause a leak. Therefore I am closing this as “works for me”.

Working backwards from all the spurious version changes (I wish people wouldn’t do that!), I figure that this was originally opened against Python 2.4 only. It is possible that the bug got fixed in the meantime.

For the question of coping with exceptions from handle_error(), the change proposed in Issue 25139 should cover that.
History
Date User Action Args
2016-02-19 22:48:14martin.pantersetstatus: open -> closed
resolution: works for me
messages: + msg260533
2015-02-09 10:48:57martin.pantersetnosy: + martin.panter
messages: + msg235599
2015-01-17 03:46:13martin.pantersettitle: FD leak in SocketServer -> FD leak in SocketServer when request handler throws exception
2014-02-03 19:13:37BreamoreBoysetnosy: - BreamoreBoy
2012-03-13 09:14:05mcjeffsetmessages: + msg155585
2011-06-12 22:25:17terry.reedysetversions: + Python 3.3, - Python 3.1
2011-03-14 04:50:24mcjeffsetnosy: draghuram, giampaolo.rodola, luke-jr, mcjeff, BreamoreBoy
messages: + msg130788
2010-09-18 14:51:49mcjeffsetmessages: + msg116788
2010-09-18 14:37:10BreamoreBoysetversions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.6
nosy: + BreamoreBoy

messages: + msg116786

type: behavior
stage: test needed
2010-05-03 04:17:24mcjeffsetnosy: + mcjeff
messages: + msg104814
2008-06-04 02:14:23giampaolo.rodolasetnosy: + giampaolo.rodola
2008-01-20 19:54:28christian.heimessetpriority: high
versions: + Python 2.6, - Python 2.4
2007-11-13 20:45:42draghuramsetnosy: + draghuram
messages: + msg57471
2007-11-12 12:27:34luke-jrcreate