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.

10 files changed, 189 insertions, 25 deletions

diff --git a/Lib/socket.py b/Lib/socket.py
index 6af1964..2dc9736 100644
--- a/Lib/socket.py
+++ b/Lib/socket.py
@@ -108,7 +108,7 @@ class socket(_socket.socket):
         if s.startswith("<socket object"):
             s = "<%s.%s%s%s" % (self.__class__.__module__,
                                 self.__class__.__name__,
-                                (self._closed and " [closed] ") or "",
+                                getattr(self, '_closed', False) and " [closed] " or "",
                                 s[7:])
         return s
 
diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
index 8784e34..7ce9753 100644
--- a/Lib/test/test_io.py
+++ b/Lib/test/test_io.py
@@ -29,6 +29,7 @@ import weakref
 import abc
 import signal
 import errno
+import warnings
 from itertools import cycle, count
 from collections import deque
 from test import support
@@ -2525,6 +2526,46 @@ class MiscIOTest(unittest.TestCase):
         # baseline "io" module.
         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):
     io = io
 
diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index 699efc0..e20364d 100644
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -706,6 +706,23 @@ class GeneralModuleTests(unittest.TestCase):
     def test_sendall_interrupted_with_timeout(self):
         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.')
 class BasicTCPTest(SocketConnectedTest):
diff --git a/Lib/xml/etree/ElementTree.py b/Lib/xml/etree/ElementTree.py
index ecc8ea7..9f5717e 100644
--- a/Lib/xml/etree/ElementTree.py
+++ b/Lib/xml/etree/ElementTree.py
@@ -662,17 +662,23 @@ class ElementTree:
     # @exception ParseError If the parser fails to parse the document.
 
     def parse(self, source, parser=None):
+        close_source = False
         if not hasattr(source, "read"):
             source = open(source, "rb")
-        if not parser:
-            parser = XMLParser(target=TreeBuilder())
-        while 1:
-            data = source.read(65536)
-            if not data:
-                break
-            parser.feed(data)
-        self._root = parser.close()
-        return self._root
+            close_source = True
+        try:
+            if not parser:
+                parser = XMLParser(target=TreeBuilder())
+            while 1:
+                data = source.read(65536)
+                if not data:
+                    break
+                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
@@ -1226,16 +1232,19 @@ def parse(source, parser=None):
 # @return A (event, elem) iterator.
 
 def iterparse(source, events=None, parser=None):
+    close_source = False
     if not hasattr(source, "read"):
         source = open(source, "rb")
+        close_source = True
     if not parser:
         parser = XMLParser(target=TreeBuilder())
-    return _IterParseIterator(source, events, parser)
+    return _IterParseIterator(source, events, parser, close_source)
 
 class _IterParseIterator:
 
-    def __init__(self, source, events, parser):
+    def __init__(self, source, events, parser, close_source=False):
         self._file = source
+        self._close_file = close_source
         self._events = []
         self._index = 0
         self.root = self._root = None
@@ -1282,6 +1291,8 @@ class _IterParseIterator:
             except IndexError:
                 if self._parser is None:
                     self.root = self._root
+                    if self._close_file:
+                        self._file.close()
                     raise StopIteration
                 # load event buffer
                 del self._events[:]
diff --git a/Misc/NEWS b/Misc/NEWS
index 7fd0d7b..6b63779 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -54,6 +54,10 @@ Core and Builtins
 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
   extensions and create files with holes when extracting sparse members.
 
diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c
index 876ab3a..851eed2 100644
--- a/Modules/_elementtree.c
+++ b/Modules/_elementtree.c
@@ -2946,19 +2946,25 @@ PyInit__elementtree(void)
 
         "class ElementTree(ET.ElementTree):\n" /* public */
         "  def parse(self, source, parser=None):\n"
+        "    close_source = False\n"
         "    if not hasattr(source, 'read'):\n"
         "      source = open(source, 'rb')\n"
-        "    if parser is not None:\n"
-        "      while 1:\n"
-        "        data = source.read(65536)\n"
-        "        if not data:\n"
-        "          break\n"
-        "        parser.feed(data)\n"
-        "      self._root = parser.close()\n"
-        "    else:\n" 
-        "      parser = cElementTree.XMLParser()\n"
-        "      self._root = parser._parse(source)\n"
-        "    return self._root\n"
+        "      close_source = True\n"
+        "    try:\n"
+        "      if parser is not None:\n"
+        "        while 1:\n"
+        "          data = source.read(65536)\n"
+        "          if not data:\n"
+        "            break\n"
+        "          parser.feed(data)\n"
+        "        self._root = parser.close()\n"
+        "      else:\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"
 
         "def iter(node, tag=None):\n" /* helper */
@@ -2988,8 +2994,10 @@ PyInit__elementtree(void)
         "class iterparse:\n"
         " root = None\n"
         " def __init__(self, file, events=None):\n"
+        "  self._close_file = False\n"
         "  if not hasattr(file, 'read'):\n"
         "    file = open(file, 'rb')\n"
+        "    self._close_file = True\n"
         "  self._file = file\n"
         "  self._events = []\n"
         "  self._index = 0\n"
@@ -3004,6 +3012,8 @@ PyInit__elementtree(void)
         "    except IndexError:\n"
         "      if self._parser is None:\n"
         "        self.root = self._root\n"
+        "        if self._close_file:\n"
+        "          self._file.close()\n"
         "        raise StopIteration\n"
         "      # load event buffer\n"
         "      del self._events[:]\n"
diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c
index 615e644..3045169 100644
--- a/Modules/_io/bufferedio.c
+++ b/Modules/_io/bufferedio.c
@@ -197,6 +197,7 @@ typedef struct {
     int detached;
     int readable;
     int writable;
+    int deallocating;
     
     /* True if this is a vanilla Buffered object (rather than a user derived
        class) *and* the raw stream is a vanilla FileIO object. */
@@ -342,6 +343,7 @@ typedef struct {
 static void
 buffered_dealloc(buffered *self)
 {
+    self->deallocating = 1;
     if (self->ok && _PyIOBase_finalize((PyObject *) self) < 0)
         return;
     _PyObject_GC_UNTRACK(self);
@@ -382,6 +384,23 @@ buffered_clear(buffered *self)
     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
  * 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);
         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 */
     LEAVE_BUFFERED(self)
     res = PyObject_CallMethodObjArgs((PyObject *)self, _PyIO_str_flush, NULL);
@@ -1461,6 +1488,7 @@ static PyMethodDef bufferedreader_methods[] = {
     {"writable", (PyCFunction)buffered_writable, METH_NOARGS},
     {"fileno", (PyCFunction)buffered_fileno, METH_NOARGS},
     {"isatty", (PyCFunction)buffered_isatty, METH_NOARGS},
+    {"_dealloc_warn", (PyCFunction)buffered_dealloc_warn, METH_O},
 
     {"read", (PyCFunction)buffered_read, METH_VARARGS},
     {"peek", (PyCFunction)buffered_peek, METH_VARARGS},
@@ -1843,6 +1871,7 @@ static PyMethodDef bufferedwriter_methods[] = {
     {"writable", (PyCFunction)buffered_writable, METH_NOARGS},
     {"fileno", (PyCFunction)buffered_fileno, METH_NOARGS},
     {"isatty", (PyCFunction)buffered_isatty, METH_NOARGS},
+    {"_dealloc_warn", (PyCFunction)buffered_dealloc_warn, METH_O},
 
     {"write", (PyCFunction)bufferedwriter_write, METH_VARARGS},
     {"truncate", (PyCFunction)buffered_truncate, METH_VARARGS},
@@ -2227,6 +2256,7 @@ static PyMethodDef bufferedrandom_methods[] = {
     {"writable", (PyCFunction)buffered_writable, METH_NOARGS},
     {"fileno", (PyCFunction)buffered_fileno, METH_NOARGS},
     {"isatty", (PyCFunction)buffered_isatty, METH_NOARGS},
+    {"_dealloc_warn", (PyCFunction)buffered_dealloc_warn, METH_O},
 
     {"flush", (PyCFunction)buffered_flush, METH_NOARGS},
 
@@ -2296,4 +2326,3 @@ PyTypeObject PyBufferedRandom_Type = {
     0,                          /* tp_alloc */
     PyType_GenericNew,          /* tp_new */
 };
-
diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c
index 74009e3..16b98d6 100644
--- a/Modules/_io/fileio.c
+++ b/Modules/_io/fileio.c
@@ -2,6 +2,7 @@
 
 #define PY_SSIZE_T_CLEAN
 #include "Python.h"
+#include "structmember.h"
 #ifdef HAVE_SYS_TYPES_H
 #include <sys/types.h>
 #endif
@@ -55,6 +56,7 @@ typedef struct {
     unsigned int writable : 1;
     signed int seekable : 2; /* -1 means unknown */
     unsigned int closefd : 1;
+    unsigned int deallocating: 1;
     PyObject *weakreflist;
     PyObject *dict;
 } fileio;
@@ -69,6 +71,26 @@ _PyFileIO_closed(PyObject *self)
     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 *
 portable_lseek(int fd, PyObject *posobj, int whence);
 
@@ -110,6 +132,13 @@ fileio_close(fileio *self)
         self->fd = -1;
         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);
     if (errno < 0)
         return NULL;
@@ -399,6 +428,7 @@ fileio_clear(fileio *self)
 static void
 fileio_dealloc(fileio *self)
 {
+    self->deallocating = 1;
     if (_PyIOBase_finalize((PyObject *) self) < 0)
         return;
     _PyObject_GC_UNTRACK(self);
@@ -1008,6 +1038,7 @@ static PyMethodDef fileio_methods[] = {
     {"writable", (PyCFunction)fileio_writable, METH_NOARGS,      writable_doc},
     {"fileno",   (PyCFunction)fileio_fileno,   METH_NOARGS,      fileno_doc},
     {"isatty",   (PyCFunction)fileio_isatty,   METH_NOARGS,      isatty_doc},
+    {"_dealloc_warn", (PyCFunction)fileio_dealloc_warn, METH_O, NULL},
     {NULL,           NULL}             /* sentinel */
 };
 
diff --git a/Modules/_io/textio.c b/Modules/_io/textio.c
index 08827b9..e222067 100644
--- a/Modules/_io/textio.c
+++ b/Modules/_io/textio.c
@@ -658,6 +658,7 @@ typedef struct
     char writetranslate;
     char seekable;
     char telling;
+    char deallocating;
     /* Specialized encoding func (see below) */
     encodefunc_t encodefunc;
     /* Whether or not it's the start of the stream */
@@ -1094,6 +1095,7 @@ _textiowrapper_clear(textio *self)
 static void
 textiowrapper_dealloc(textio *self)
 {
+    self->deallocating = 1;
     if (_textiowrapper_clear(self) < 0)
         return;
     _PyObject_GC_UNTRACK(self);
@@ -2410,6 +2412,13 @@ textiowrapper_close(textio *self, PyObject *args)
         Py_RETURN_NONE; /* stream already closed */
     }
     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);
         if (res == NULL) {
             return NULL;
diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
index eeb9304..fdbf7ee 100644
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -2941,8 +2941,20 @@ static PyMemberDef sock_memberlist[] = {
 static void
 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);
+        Py_REFCNT(s) = old_refcount;
+    }
     Py_TYPE(s)->tp_free((PyObject *)s);
 }