Issue #10093: ResourceWarnings are now issued when files and sockets are

deallocated without explicit closing.  These warnings are silenced by
default, except in pydebug mode.
This commit is contained in:
Antoine Pitrou 2010-10-29 10:38:18 +00:00
parent 9cbdd75ec5
commit e033e06db0
10 changed files with 189 additions and 25 deletions

View file

@ -108,7 +108,7 @@ class socket(_socket.socket):
if s.startswith("<socket object"): if s.startswith("<socket object"):
s = "<%s.%s%s%s" % (self.__class__.__module__, s = "<%s.%s%s%s" % (self.__class__.__module__,
self.__class__.__name__, self.__class__.__name__,
(self._closed and " [closed] ") or "", getattr(self, '_closed', False) and " [closed] " or "",
s[7:]) s[7:])
return s return s

View file

@ -29,6 +29,7 @@ import weakref
import abc import abc
import signal import signal
import errno import errno
import warnings
from itertools import cycle, count from itertools import cycle, count
from collections import deque from collections import deque
from test import support from test import support
@ -2525,6 +2526,46 @@ class MiscIOTest(unittest.TestCase):
# baseline "io" module. # baseline "io" module.
self._check_abc_inheritance(io) self._check_abc_inheritance(io)
def _check_warn_on_dealloc(self, *args, **kwargs):
f = open(*args, **kwargs)
r = repr(f)
with self.assertWarns(ResourceWarning) as cm:
f = None
support.gc_collect()
self.assertIn(r, str(cm.warning.args[0]))
def test_warn_on_dealloc(self):
self._check_warn_on_dealloc(support.TESTFN, "wb", buffering=0)
self._check_warn_on_dealloc(support.TESTFN, "wb")
self._check_warn_on_dealloc(support.TESTFN, "w")
def _check_warn_on_dealloc_fd(self, *args, **kwargs):
fds = []
try:
r, w = os.pipe()
fds += r, w
self._check_warn_on_dealloc(r, *args, **kwargs)
# When using closefd=False, there's no warning
r, w = os.pipe()
fds += r, w
with warnings.catch_warnings(record=True) as recorded:
open(r, *args, closefd=False, **kwargs)
support.gc_collect()
self.assertEqual(recorded, [])
finally:
for fd in fds:
try:
os.close(fd)
except EnvironmentError as e:
if e.errno != errno.EBADF:
raise
def test_warn_on_dealloc_fd(self):
self._check_warn_on_dealloc_fd("rb", buffering=0)
self._check_warn_on_dealloc_fd("rb")
self._check_warn_on_dealloc_fd("r")
class CMiscIOTest(MiscIOTest): class CMiscIOTest(MiscIOTest):
io = io io = io

View file

@ -706,6 +706,23 @@ class GeneralModuleTests(unittest.TestCase):
def test_sendall_interrupted_with_timeout(self): def test_sendall_interrupted_with_timeout(self):
self.check_sendall_interrupted(True) self.check_sendall_interrupted(True)
def test_dealloc_warn(self):
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
r = repr(sock)
with self.assertWarns(ResourceWarning) as cm:
sock = None
support.gc_collect()
self.assertIn(r, str(cm.warning.args[0]))
# An open socket file object gets dereferenced after the socket
sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
f = sock.makefile('rb')
r = repr(sock)
sock = None
support.gc_collect()
with self.assertWarns(ResourceWarning):
f = None
support.gc_collect()
@unittest.skipUnless(thread, 'Threading required for this test.') @unittest.skipUnless(thread, 'Threading required for this test.')
class BasicTCPTest(SocketConnectedTest): class BasicTCPTest(SocketConnectedTest):

View file

@ -662,17 +662,23 @@ class ElementTree:
# @exception ParseError If the parser fails to parse the document. # @exception ParseError If the parser fails to parse the document.
def parse(self, source, parser=None): def parse(self, source, parser=None):
close_source = False
if not hasattr(source, "read"): if not hasattr(source, "read"):
source = open(source, "rb") source = open(source, "rb")
if not parser: close_source = True
parser = XMLParser(target=TreeBuilder()) try:
while 1: if not parser:
data = source.read(65536) parser = XMLParser(target=TreeBuilder())
if not data: while 1:
break data = source.read(65536)
parser.feed(data) if not data:
self._root = parser.close() break
return self._root parser.feed(data)
self._root = parser.close()
return self._root
finally:
if close_source:
source.close()
## ##
# Creates a tree iterator for the root element. The iterator loops # Creates a tree iterator for the root element. The iterator loops
@ -1226,16 +1232,19 @@ def parse(source, parser=None):
# @return A (event, elem) iterator. # @return A (event, elem) iterator.
def iterparse(source, events=None, parser=None): def iterparse(source, events=None, parser=None):
close_source = False
if not hasattr(source, "read"): if not hasattr(source, "read"):
source = open(source, "rb") source = open(source, "rb")
close_source = True
if not parser: if not parser:
parser = XMLParser(target=TreeBuilder()) parser = XMLParser(target=TreeBuilder())
return _IterParseIterator(source, events, parser) return _IterParseIterator(source, events, parser, close_source)
class _IterParseIterator: class _IterParseIterator:
def __init__(self, source, events, parser): def __init__(self, source, events, parser, close_source=False):
self._file = source self._file = source
self._close_file = close_source
self._events = [] self._events = []
self._index = 0 self._index = 0
self.root = self._root = None self.root = self._root = None
@ -1282,6 +1291,8 @@ class _IterParseIterator:
except IndexError: except IndexError:
if self._parser is None: if self._parser is None:
self.root = self._root self.root = self._root
if self._close_file:
self._file.close()
raise StopIteration raise StopIteration
# load event buffer # load event buffer
del self._events[:] del self._events[:]

View file

@ -54,6 +54,10 @@ Core and Builtins
Library Library
------- -------
- Issue #10093: ResourceWarnings are now issued when files and sockets are
deallocated without explicit closing. These warnings are silenced by
default, except in pydebug mode.
- tarfile.py: Add support for all missing variants of the GNU sparse - tarfile.py: Add support for all missing variants of the GNU sparse
extensions and create files with holes when extracting sparse members. extensions and create files with holes when extracting sparse members.

View file

@ -2946,19 +2946,25 @@ PyInit__elementtree(void)
"class ElementTree(ET.ElementTree):\n" /* public */ "class ElementTree(ET.ElementTree):\n" /* public */
" def parse(self, source, parser=None):\n" " def parse(self, source, parser=None):\n"
" close_source = False\n"
" if not hasattr(source, 'read'):\n" " if not hasattr(source, 'read'):\n"
" source = open(source, 'rb')\n" " source = open(source, 'rb')\n"
" if parser is not None:\n" " close_source = True\n"
" while 1:\n" " try:\n"
" data = source.read(65536)\n" " if parser is not None:\n"
" if not data:\n" " while 1:\n"
" break\n" " data = source.read(65536)\n"
" parser.feed(data)\n" " if not data:\n"
" self._root = parser.close()\n" " break\n"
" else:\n" " parser.feed(data)\n"
" parser = cElementTree.XMLParser()\n" " self._root = parser.close()\n"
" self._root = parser._parse(source)\n" " else:\n"
" return self._root\n" " parser = cElementTree.XMLParser()\n"
" self._root = parser._parse(source)\n"
" return self._root\n"
" finally:\n"
" if close_source:\n"
" source.close()\n"
"cElementTree.ElementTree = ElementTree\n" "cElementTree.ElementTree = ElementTree\n"
"def iter(node, tag=None):\n" /* helper */ "def iter(node, tag=None):\n" /* helper */
@ -2988,8 +2994,10 @@ PyInit__elementtree(void)
"class iterparse:\n" "class iterparse:\n"
" root = None\n" " root = None\n"
" def __init__(self, file, events=None):\n" " def __init__(self, file, events=None):\n"
" self._close_file = False\n"
" if not hasattr(file, 'read'):\n" " if not hasattr(file, 'read'):\n"
" file = open(file, 'rb')\n" " file = open(file, 'rb')\n"
" self._close_file = True\n"
" self._file = file\n" " self._file = file\n"
" self._events = []\n" " self._events = []\n"
" self._index = 0\n" " self._index = 0\n"
@ -3004,6 +3012,8 @@ PyInit__elementtree(void)
" except IndexError:\n" " except IndexError:\n"
" if self._parser is None:\n" " if self._parser is None:\n"
" self.root = self._root\n" " self.root = self._root\n"
" if self._close_file:\n"
" self._file.close()\n"
" raise StopIteration\n" " raise StopIteration\n"
" # load event buffer\n" " # load event buffer\n"
" del self._events[:]\n" " del self._events[:]\n"

View file

@ -197,6 +197,7 @@ typedef struct {
int detached; int detached;
int readable; int readable;
int writable; int writable;
int deallocating;
/* True if this is a vanilla Buffered object (rather than a user derived /* True if this is a vanilla Buffered object (rather than a user derived
class) *and* the raw stream is a vanilla FileIO object. */ class) *and* the raw stream is a vanilla FileIO object. */
@ -342,6 +343,7 @@ typedef struct {
static void static void
buffered_dealloc(buffered *self) buffered_dealloc(buffered *self)
{ {
self->deallocating = 1;
if (self->ok && _PyIOBase_finalize((PyObject *) self) < 0) if (self->ok && _PyIOBase_finalize((PyObject *) self) < 0)
return; return;
_PyObject_GC_UNTRACK(self); _PyObject_GC_UNTRACK(self);
@ -382,6 +384,23 @@ buffered_clear(buffered *self)
return 0; return 0;
} }
/* Because this can call arbitrary code, it shouldn't be called when
the refcount is 0 (that is, not directly from tp_dealloc unless
the refcount has been temporarily re-incremented). */
PyObject *
buffered_dealloc_warn(buffered *self, PyObject *source)
{
if (self->ok && self->raw) {
PyObject *r;
r = PyObject_CallMethod(self->raw, "_dealloc_warn", "O", source);
if (r)
Py_DECREF(r);
else
PyErr_Clear();
}
Py_RETURN_NONE;
}
/* /*
* _BufferedIOMixin methods * _BufferedIOMixin methods
* This is not a class, just a collection of methods that will be reused * This is not a class, just a collection of methods that will be reused
@ -435,6 +454,14 @@ buffered_close(buffered *self, PyObject *args)
Py_INCREF(res); Py_INCREF(res);
goto end; goto end;
} }
if (self->deallocating) {
PyObject *r = buffered_dealloc_warn(self, (PyObject *) self);
if (r)
Py_DECREF(r);
else
PyErr_Clear();
}
/* flush() will most probably re-take the lock, so drop it first */ /* flush() will most probably re-take the lock, so drop it first */
LEAVE_BUFFERED(self) LEAVE_BUFFERED(self)
res = PyObject_CallMethodObjArgs((PyObject *)self, _PyIO_str_flush, NULL); res = PyObject_CallMethodObjArgs((PyObject *)self, _PyIO_str_flush, NULL);
@ -1461,6 +1488,7 @@ static PyMethodDef bufferedreader_methods[] = {
{"writable", (PyCFunction)buffered_writable, METH_NOARGS}, {"writable", (PyCFunction)buffered_writable, METH_NOARGS},
{"fileno", (PyCFunction)buffered_fileno, METH_NOARGS}, {"fileno", (PyCFunction)buffered_fileno, METH_NOARGS},
{"isatty", (PyCFunction)buffered_isatty, METH_NOARGS}, {"isatty", (PyCFunction)buffered_isatty, METH_NOARGS},
{"_dealloc_warn", (PyCFunction)buffered_dealloc_warn, METH_O},
{"read", (PyCFunction)buffered_read, METH_VARARGS}, {"read", (PyCFunction)buffered_read, METH_VARARGS},
{"peek", (PyCFunction)buffered_peek, METH_VARARGS}, {"peek", (PyCFunction)buffered_peek, METH_VARARGS},
@ -1843,6 +1871,7 @@ static PyMethodDef bufferedwriter_methods[] = {
{"writable", (PyCFunction)buffered_writable, METH_NOARGS}, {"writable", (PyCFunction)buffered_writable, METH_NOARGS},
{"fileno", (PyCFunction)buffered_fileno, METH_NOARGS}, {"fileno", (PyCFunction)buffered_fileno, METH_NOARGS},
{"isatty", (PyCFunction)buffered_isatty, METH_NOARGS}, {"isatty", (PyCFunction)buffered_isatty, METH_NOARGS},
{"_dealloc_warn", (PyCFunction)buffered_dealloc_warn, METH_O},
{"write", (PyCFunction)bufferedwriter_write, METH_VARARGS}, {"write", (PyCFunction)bufferedwriter_write, METH_VARARGS},
{"truncate", (PyCFunction)buffered_truncate, METH_VARARGS}, {"truncate", (PyCFunction)buffered_truncate, METH_VARARGS},
@ -2227,6 +2256,7 @@ static PyMethodDef bufferedrandom_methods[] = {
{"writable", (PyCFunction)buffered_writable, METH_NOARGS}, {"writable", (PyCFunction)buffered_writable, METH_NOARGS},
{"fileno", (PyCFunction)buffered_fileno, METH_NOARGS}, {"fileno", (PyCFunction)buffered_fileno, METH_NOARGS},
{"isatty", (PyCFunction)buffered_isatty, METH_NOARGS}, {"isatty", (PyCFunction)buffered_isatty, METH_NOARGS},
{"_dealloc_warn", (PyCFunction)buffered_dealloc_warn, METH_O},
{"flush", (PyCFunction)buffered_flush, METH_NOARGS}, {"flush", (PyCFunction)buffered_flush, METH_NOARGS},
@ -2296,4 +2326,3 @@ PyTypeObject PyBufferedRandom_Type = {
0, /* tp_alloc */ 0, /* tp_alloc */
PyType_GenericNew, /* tp_new */ PyType_GenericNew, /* tp_new */
}; };

View file

@ -2,6 +2,7 @@
#define PY_SSIZE_T_CLEAN #define PY_SSIZE_T_CLEAN
#include "Python.h" #include "Python.h"
#include "structmember.h"
#ifdef HAVE_SYS_TYPES_H #ifdef HAVE_SYS_TYPES_H
#include <sys/types.h> #include <sys/types.h>
#endif #endif
@ -55,6 +56,7 @@ typedef struct {
unsigned int writable : 1; unsigned int writable : 1;
signed int seekable : 2; /* -1 means unknown */ signed int seekable : 2; /* -1 means unknown */
unsigned int closefd : 1; unsigned int closefd : 1;
unsigned int deallocating: 1;
PyObject *weakreflist; PyObject *weakreflist;
PyObject *dict; PyObject *dict;
} fileio; } fileio;
@ -69,6 +71,26 @@ _PyFileIO_closed(PyObject *self)
return ((fileio *)self)->fd < 0; return ((fileio *)self)->fd < 0;
} }
/* Because this can call arbitrary code, it shouldn't be called when
the refcount is 0 (that is, not directly from tp_dealloc unless
the refcount has been temporarily re-incremented). */
static PyObject *
fileio_dealloc_warn(fileio *self, PyObject *source)
{
if (self->fd >= 0 && self->closefd) {
PyObject *exc, *val, *tb;
PyErr_Fetch(&exc, &val, &tb);
if (PyErr_WarnFormat(PyExc_ResourceWarning, 1,
"unclosed file %R", source)) {
/* Spurious errors can appear at shutdown */
if (PyErr_ExceptionMatches(PyExc_Warning))
PyErr_WriteUnraisable((PyObject *) self);
}
PyErr_Restore(exc, val, tb);
}
Py_RETURN_NONE;
}
static PyObject * static PyObject *
portable_lseek(int fd, PyObject *posobj, int whence); portable_lseek(int fd, PyObject *posobj, int whence);
@ -110,6 +132,13 @@ fileio_close(fileio *self)
self->fd = -1; self->fd = -1;
Py_RETURN_NONE; Py_RETURN_NONE;
} }
if (self->deallocating) {
PyObject *r = fileio_dealloc_warn(self, (PyObject *) self);
if (r)
Py_DECREF(r);
else
PyErr_Clear();
}
errno = internal_close(self); errno = internal_close(self);
if (errno < 0) if (errno < 0)
return NULL; return NULL;
@ -399,6 +428,7 @@ fileio_clear(fileio *self)
static void static void
fileio_dealloc(fileio *self) fileio_dealloc(fileio *self)
{ {
self->deallocating = 1;
if (_PyIOBase_finalize((PyObject *) self) < 0) if (_PyIOBase_finalize((PyObject *) self) < 0)
return; return;
_PyObject_GC_UNTRACK(self); _PyObject_GC_UNTRACK(self);
@ -1008,6 +1038,7 @@ static PyMethodDef fileio_methods[] = {
{"writable", (PyCFunction)fileio_writable, METH_NOARGS, writable_doc}, {"writable", (PyCFunction)fileio_writable, METH_NOARGS, writable_doc},
{"fileno", (PyCFunction)fileio_fileno, METH_NOARGS, fileno_doc}, {"fileno", (PyCFunction)fileio_fileno, METH_NOARGS, fileno_doc},
{"isatty", (PyCFunction)fileio_isatty, METH_NOARGS, isatty_doc}, {"isatty", (PyCFunction)fileio_isatty, METH_NOARGS, isatty_doc},
{"_dealloc_warn", (PyCFunction)fileio_dealloc_warn, METH_O, NULL},
{NULL, NULL} /* sentinel */ {NULL, NULL} /* sentinel */
}; };

View file

@ -658,6 +658,7 @@ typedef struct
char writetranslate; char writetranslate;
char seekable; char seekable;
char telling; char telling;
char deallocating;
/* Specialized encoding func (see below) */ /* Specialized encoding func (see below) */
encodefunc_t encodefunc; encodefunc_t encodefunc;
/* Whether or not it's the start of the stream */ /* Whether or not it's the start of the stream */
@ -1094,6 +1095,7 @@ _textiowrapper_clear(textio *self)
static void static void
textiowrapper_dealloc(textio *self) textiowrapper_dealloc(textio *self)
{ {
self->deallocating = 1;
if (_textiowrapper_clear(self) < 0) if (_textiowrapper_clear(self) < 0)
return; return;
_PyObject_GC_UNTRACK(self); _PyObject_GC_UNTRACK(self);
@ -2410,6 +2412,13 @@ textiowrapper_close(textio *self, PyObject *args)
Py_RETURN_NONE; /* stream already closed */ Py_RETURN_NONE; /* stream already closed */
} }
else { else {
if (self->deallocating) {
res = PyObject_CallMethod(self->buffer, "_dealloc_warn", "O", self);
if (res)
Py_DECREF(res);
else
PyErr_Clear();
}
res = PyObject_CallMethod((PyObject *)self, "flush", NULL); res = PyObject_CallMethod((PyObject *)self, "flush", NULL);
if (res == NULL) { if (res == NULL) {
return NULL; return NULL;

View file

@ -2941,8 +2941,20 @@ static PyMemberDef sock_memberlist[] = {
static void static void
sock_dealloc(PySocketSockObject *s) sock_dealloc(PySocketSockObject *s)
{ {
if (s->sock_fd != -1) if (s->sock_fd != -1) {
PyObject *exc, *val, *tb;
Py_ssize_t old_refcount = Py_REFCNT(s);
++Py_REFCNT(s);
PyErr_Fetch(&exc, &val, &tb);
if (PyErr_WarnFormat(PyExc_ResourceWarning, 1,
"unclosed %R", s))
/* Spurious errors can appear at shutdown */
if (PyErr_ExceptionMatches(PyExc_Warning))
PyErr_WriteUnraisable((PyObject *) s);
PyErr_Restore(exc, val, tb);
(void) SOCKETCLOSE(s->sock_fd); (void) SOCKETCLOSE(s->sock_fd);
Py_REFCNT(s) = old_refcount;
}
Py_TYPE(s)->tp_free((PyObject *)s); Py_TYPE(s)->tp_free((PyObject *)s);
} }