bottlepy / bottle

bottle.py is a fast and simple micro-framework for python web-applications.
http://bottlepy.org/
MIT License
8.37k stars 1.46k forks source link

HTTP POST: ResourceWarning: unclosed file <_io.BufferedRandom name=36> #1206

Closed mcr-ksh closed 2 years ago

mcr-ksh commented 4 years ago

There is a resource leek that I cannot determine from my code. I guess it's from bottle.

Bottle v0.12.18 server starting up (using WSGIRefServer())...
Listening on http://irsdev:5000/
Hit Ctrl-C to quit.

On Fileupload (via tracemalloc):

/devel/anaconda3/envs/asdf/bin/bottle.py:1672: ResourceWarning: unclosed file <_io.BufferedRandom name=36> def fset(self, value): ls.var = value Object allocated at (most recent call last): File "/devel/anaconda3/envs/asdf/lib/python3.7/tempfile.py", lineno 622 newline=newline, encoding=encoding)

Function:

@route('/api/<threshold:float>', method='POST')
def func(threshold=0.5):

Command: curl -X POST -F 'file=@/tmp/pic.jpg' http://irsdev:5000/api/0.5

x-yuri commented 4 years ago

At some point you probably access request.body. And if the body is too big it puts it into a temporary file (opens a file). From what I can see you're responsible for closing the file, e.g.:

with request.body as body:
    # process body
# here body is closed

Unless you do, you get the warning. Such warnings are displayed only in debug mode. To get more lines of traceback, run it with PYTHONTRACEMALLOC=N, where N is the desired number of lines of the traceback.

Similar issue: #665.

defnull commented 2 years ago

Turns out using NamedTemporaryFile instead of TemporaryFile fixes the ResourceWarning in most cases because NamedTemporaryFile (indirectly) implements __del__ to explicitly close (and remove) the file. This is not needed for TemporaryFile because these are unlinked immediately, which is why these are still technically 'open' when the garbage collector collects the file, which in turn triggers the warning.

Only downside is that on a hard crash, a temporary file may remain on disk, so this is not an ideal solution. Deprecating a request.body that lives longer than the request and switching back to TemporaryFile might be the cleaner option. Until then, I'll consider this fixed (in master). I'll keep one of the related issues open.

tobi1220 commented 10 months ago

I know that this is old, but for anyone who stumbles upon this: even if you're not directly accessing request.body but rather referencing your uploaded file with request.files.get("NAME").file , you still need to close the body with request.body.close(), otherwise you'll also get this warning.