gh-128277: remove unnecessary critical section from `socket.close` (#128305)

Remove unnecessary critical section from `socket.close` as it now uses relaxed atomics for `sock_fd`.

diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py
index aac213e36aecf0fc5b56376e12dfa6dcf07ce146..faf326d9164e1bbf8ad3ec8b7b154b1947a07660 100644 (file)
--- a/Lib/test/test_socket.py
+++ b/Lib/test/test_socket.py
@@ -7075,6 +7075,26 @@ class SendRecvFdsTests(unittest.TestCase):
             self.assertEqual(data,  str(index).encode())
 
 
+class FreeThreadingTests(unittest.TestCase):
+
+    def test_close_detach_race(self):
+        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+
+        def close():
+            for _ in range(1000):
+                s.close()
+
+        def detach():
+            for _ in range(1000):
+                s.detach()
+
+        t1 = threading.Thread(target=close)
+        t2 = threading.Thread(target=detach)
+
+        with threading_helper.start_threads([t1, t2]):
+            pass
+
+
 def setUpModule():
     thread_info = threading_helper.threading_setup()
     unittest.addModuleCleanup(threading_helper.threading_cleanup, *thread_info)

diff --git a/Modules/clinic/socketmodule.c.h b/Modules/clinic/socketmodule.c.h
index db1a28b86c8773f41d6eeaa939a4402b120a2d6c..2152f288a9722f899a24a20202a23ed7682e0390 100644 (file)
--- a/Modules/clinic/socketmodule.c.h
+++ b/Modules/clinic/socketmodule.c.h
@@ -6,7 +6,6 @@ preserve
 #  include "pycore_gc.h"          // PyGC_Head
 #  include "pycore_runtime.h"     // _Py_ID()
 #endif
-#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION()
 #include "pycore_modsupport.h"    // _PyArg_UnpackKeywords()
 
 PyDoc_STRVAR(_socket_socket_close__doc__,
@@ -26,13 +25,7 @@ _socket_socket_close_impl(PySocketSockObject *s);
 static PyObject *
 _socket_socket_close(PySocketSockObject *s, PyObject *Py_UNUSED(ignored))
 {
-    PyObject *return_value = NULL;
-
-    Py_BEGIN_CRITICAL_SECTION(s);
-    return_value = _socket_socket_close_impl(s);
-    Py_END_CRITICAL_SECTION();
-
-    return return_value;
+    return _socket_socket_close_impl(s);
 }
 
 static int
@@ -287,4 +280,4 @@ exit:
 #ifndef _SOCKET_SOCKET_IF_NAMETOINDEX_METHODDEF
     #define _SOCKET_SOCKET_IF_NAMETOINDEX_METHODDEF
 #endif /* !defined(_SOCKET_SOCKET_IF_NAMETOINDEX_METHODDEF) */
-/*[clinic end generated code: output=59c36bb31b05de68 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=3e612e8df1c322dd input=a9049054013a1b77]*/

diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c
index efc8be3ab592ba772ac858d752fd1ba9d8ba5b5a..e70aa304f2f3a391c703bcbf30e12b6e9890c7b5 100644 (file)
--- a/Modules/socketmodule.c
+++ b/Modules/socketmodule.c
@@ -3386,7 +3386,6 @@ sockets the address is a tuple (ifname, proto [,pkttype [,hatype [,addr]]])");
    will surely fail. */
 
 /*[clinic input]
-@critical_section
 _socket.socket.close
     self as s: self(type="PySocketSockObject *")
 
@@ -3397,7 +3396,7 @@ Close the socket.  It cannot be used after this call.
 
 static PyObject *
 _socket_socket_close_impl(PySocketSockObject *s)
-/*[clinic end generated code: output=038b2418e07f6f6c input=9839a261e05bcb97]*/
+/*[clinic end generated code: output=038b2418e07f6f6c input=dc487e470e55a83c]*/
 {
     SOCKET_T fd;
     int res;