From 50ab1a3694c43b9ab6798b98d9e5983c78cb17e2 Mon Sep 17 00:00:00 2001 From: Martin Panter Date: Mon, 11 Apr 2016 00:38:12 +0000 Subject: [PATCH] Issue #26685: Raise OSError if closing a socket fails --- Doc/library/socket.rst | 4 ++++ Doc/whatsnew/3.6.rst | 4 ++++ Lib/test/test_pty.py | 1 - Lib/test/test_socket.py | 11 +++++++++++ Misc/NEWS | 2 ++ Modules/socketmodule.c | 6 +++++- 6 files changed, 26 insertions(+), 2 deletions(-) diff --git a/Doc/library/socket.rst b/Doc/library/socket.rst index c09927cf345..cd6e3105b07 100644 --- a/Doc/library/socket.rst +++ b/Doc/library/socket.rst @@ -868,6 +868,10 @@ to sockets. it is recommended to :meth:`close` them explicitly, or to use a :keyword:`with` statement around them. + .. versionchanged:: 3.6 + :exc:`OSError` is now raised if an error occurs when the underlying + :c:func:`close` call is made. + .. note:: :meth:`close()` releases the resource associated with a connection but diff --git a/Doc/whatsnew/3.6.rst b/Doc/whatsnew/3.6.rst index 9be1a9c2912..8bf28471591 100644 --- a/Doc/whatsnew/3.6.rst +++ b/Doc/whatsnew/3.6.rst @@ -514,6 +514,10 @@ Changes in the Python API * :func:`spwd.getspnam` now raises a :exc:`PermissionError` instead of :exc:`KeyError` if the user doesn't have privileges. +* The :meth:`socket.socket.close` method now raises an exception if + an error (e.g. EBADF) was reported by the underlying system call. + See :issue:`26685`. + Changes in the C API -------------------- diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index ef5e99ee269..15f88e4fcd7 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -277,7 +277,6 @@ class SmallPtyTests(unittest.TestCase): socketpair = self._socketpair() masters = [s.fileno() for s in socketpair] - os.close(masters[1]) socketpair[1].close() os.close(write_to_stdin_fd) diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py index 02bc0c0d703..982a9761263 100644 --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -1161,6 +1161,17 @@ class GeneralModuleTests(unittest.TestCase): sock.close() self.assertRaises(OSError, sock.send, b"spam") + def testCloseException(self): + sock = socket.socket() + socket.socket(fileno=sock.fileno()).close() + try: + sock.close() + except OSError as err: + # Winsock apparently raises ENOTSOCK + self.assertIn(err.errno, (errno.EBADF, errno.ENOTSOCK)) + else: + self.fail("close() should raise EBADF/ENOTSOCK") + def testNewAttributes(self): # testing .family, .type and .protocol diff --git a/Misc/NEWS b/Misc/NEWS index f7694ccb764..7ac9b341015 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -240,6 +240,8 @@ Core and Builtins Library ------- +- Issue #26685: Raise OSError if closing a socket fails. + - Issue #16329: Add .webm to mimetypes.types_map. Patch by Giampaolo Rodola'. - Issue #13952: Add .csv to mimetypes.types_map. Patch by Geoff Wilson. diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 8df735d2044..bcff00458cd 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -2576,6 +2576,7 @@ static PyObject * sock_close(PySocketSockObject *s) { SOCKET_T fd; + int res; fd = s->sock_fd; if (fd != -1) { @@ -2586,8 +2587,11 @@ sock_close(PySocketSockObject *s) http://linux.derkeiler.com/Mailing-Lists/Kernel/2005-09/3000.html for more details. */ Py_BEGIN_ALLOW_THREADS - (void) SOCKETCLOSE(fd); + res = SOCKETCLOSE(fd); Py_END_ALLOW_THREADS + if (res < 0) { + return s->errorhandler(); + } } Py_INCREF(Py_None); return Py_None;