msg192530 - (view) |
Author: Florent Xicluna (flox) *  |
Date: 2013-07-07 09:50 |
It happens when POSTing a file for example. When running the test suite: ./python.exe -m test test_cgi Or with the script attached: $ ./python test_fieldstorage.py test_fieldstorage.py:28: ResourceWarning: unclosed file <_io.BufferedRandom name=3> check('x' * 1010) # ResourceWarning test_fieldstorage.py:29: ResourceWarning: unclosed file <_io.BufferedRandom name=3> check('x' * (maxline - 1)) # ResourceWarning |
|
|
msg194940 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-12 11:22 |
It happens because if the length of data is more than 1000: def __write(self, line): """line is always bytes, not string""" if self.__file is not None: if self.__file.tell() + len(line) > 1000: self.file = self.make_file() data = self.__file.getvalue() self.file.write(data) self.__file = None ...... it will create a temporary file. Attached the patch to close the temporary file in the destructor method. About the 1000 number, should we put it in constant? Why 1000? This number is so random. For now, I just leave it as it is. |
|
|
msg194980 - (view) |
Author: Madison May (madison.may) * |
Date: 2013-08-12 18:16 |
I ran into a similar issue (see #18700) with test_cgi. ``/home/mmay/cpython/Lib/test/test_cgi.py:276: ResourceWarning: unclosed file <_io.BufferedRandom name=3>`` |
|
|
msg196009 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-08-23 19:15 |
New changeset c0e9ba7b26d5 by Brett Cannon in branch 'default': Issue #18394: Explicitly close the file object cgi.FieldStorage http://hg.python.org/cpython/rev/c0e9ba7b26d5 |
|
|
msg196010 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2013-08-23 19:17 |
I managed to write a similar patch to Vajrasky independently, so at least I know the approach is reasonable. =) I'm not backporting since it really isn't that critical; I fixed it just to turn off the ResourceWarning while running the test suite. |
|
|
msg207958 - (view) |
Author: Marcel Hellkamp (Marcel.Hellkamp) |
Date: 2014-01-12 14:32 |
This change breaks existing applications. The cgi.FieldStorage.file attribute is public and mentioned in the documentation. It even states "You can then read the data at leisure from the file attribute". Consider this example:: form = cgi.FieldStorage() fileitem = form.getfirst("userfile") if fileitem and fileitem.file: executor.submit(store_file, fileitem.file, fileitem.filename) This code is no longer safe. The garbage collector might close the file handle while it is still referenced and accessed from the worker thread. Another example is the bottle web framework. It uses cgi.FieldStorage for parsing only, extracts the valuable information and stores the result in its own data structures. The cgi.FieldStorage instance is lost. Python 3.4 breaks every single bottle application that works with file uploads. How about implementing the context manager protocol for cgi.FieldStorage to resolve this issue? |
|
|
msg208339 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2014-01-17 14:49 |
While you're right, Marcel, that code which pulls out the file object form FieldStorage would probably have the file closed from underneath it, I don't know if I agree that it's a bad thing. The FieldStorage object created that file, implicitly putting it in charge of managing it. Having it guarantee that it gets closed seems to me totally reasonable and a bug not to do so. And having Bottle break as-is doesn't sway me as this is a bug fix and so updating Bottle as part of the process to support Python 3.4 is reasonable. That use of the term "leisure" is definitely a problem, though. So I'm going to make this a bug report for updating the docs to no longer use the term "leisure" and add a versionchanged note that FieldStorage will close the 'file' attribute upon deletion. I'll also opened http://bugs.python.org/issue20289 to add context manager support to FieldStorage. But since leaving files randomly open is not good I can't bring myself to revert this change. |
|
|
msg208343 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2014-01-17 16:03 |
New changeset 13d04a8713ad by Brett Cannon in branch 'default': Issue #18394: Document that cgi.FieldStorage now cleans up after its http://hg.python.org/cpython/rev/13d04a8713ad |
|
|