Issue #26685: Raise OSError if closing a socket fails

diff --git a/Doc/library/socket.rst b/Doc/library/socket.rst
index c09927cf345df703ec935c22bbc4e2681142ddb6..cd6e3105b07d88c3c5eea4a4302b6006c9c4fb6a 100644 (file)
--- 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 9be1a9c2912d21a47b8d32b4b16f84000f5f654e..8bf284715915a4c356f4e11add1ab54f3053f240 100644 (file)
--- 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 ef5e99ee269cbafbfab8008617a9386c0a43a9a8..15f88e4fcd79848042717d3ea60741189fac4710 100644 (file)
--- 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 02bc0c0d7035cbd5a1fb630907929536d8b36293..982a9761263813b3487c1d849bce887c36dfdde4 100644 (file)
--- 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 f7694ccb764e33ff49a8c6a3fa85ebd0d5cb41d2..7ac9b341015f330a4229b2bd8d663644f8dea2e3 100644 (file)
--- 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 8df735d2044a12f97bfc852de760eb99fbe718cd..bcff00458cd49fff962f7f775365b4680ee31f2c 100644 (file)
--- 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;