Mailman 3 May 2024 - Python-checkins

[3.13] contextlib docs: Clean up redundant 'up' after 'cleanup' (GH-119872)
by zware May 31, 2024

May 31, 2024

https://github.com/python/cpython/commit/eb5e19aa685fab3832b2a44ec5867068ba… commit: eb5e19aa685fab3832b2a44ec5867068baf6e1a7 branch: 3.13 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: zware <zachary.ware(a)gmail.com> date: 2024-05-31T17:39:02Z summary: [3.13] contextlib docs: Clean up redundant 'up' after 'cleanup' (GH-119872) Reported by Michael Kass on docs@ (cherry picked from commit f3fc800d5f17b144a752a262102b750bedcdaa14) files: M Doc/library/contextlib.rst diff --git a/Doc/library/contextlib.rst b/Doc/library/contextlib.rst index 73e53aec9cbf1c..bad9da52d6a6ca 100644 --- a/Doc/library/contextlib.rst +++ b/Doc/library/contextlib.rst @@ -796,7 +796,7 @@ executing that callback:: if result: stack.pop_all() -This allows the intended cleanup up behaviour to be made explicit up front, +This allows the intended cleanup behaviour to be made explicit up front, rather than requiring a separate flag variable. If a particular application uses this pattern a lot, it can be simplified

1 0

[3.12] contextlib docs: Clean up redundant 'up' after 'cleanup' (GH-119873)
by zware May 31, 2024

May 31, 2024

https://github.com/python/cpython/commit/2f7fada58091c518fd6784e108cce83bf6… commit: 2f7fada58091c518fd6784e108cce83bf6c310ac branch: 3.12 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: zware <zachary.ware(a)gmail.com> date: 2024-05-31T17:27:57Z summary: [3.12] contextlib docs: Clean up redundant 'up' after 'cleanup' (GH-119873) Reported by Michael Kass on docs@ (cherry picked from commit f3fc800d5f17b144a752a262102b750bedcdaa14) files: M Doc/library/contextlib.rst diff --git a/Doc/library/contextlib.rst b/Doc/library/contextlib.rst index 73e53aec9cbf1c..bad9da52d6a6ca 100644 --- a/Doc/library/contextlib.rst +++ b/Doc/library/contextlib.rst @@ -796,7 +796,7 @@ executing that callback:: if result: stack.pop_all() -This allows the intended cleanup up behaviour to be made explicit up front, +This allows the intended cleanup behaviour to be made explicit up front, rather than requiring a separate flag variable. If a particular application uses this pattern a lot, it can be simplified

1 0

[3.13] gh-100117: Fix inaccuracy in documentation of the CodeObject's co_positions field. (GH-119364) (#119869)
by iritkatriel May 31, 2024

May 31, 2024

https://github.com/python/cpython/commit/24e3a8a9a6b9d99ad4b861cdcad6152952… commit: 24e3a8a9a6b9d99ad4b861cdcad6152952145a61 branch: 3.13 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: iritkatriel <1055913+iritkatriel(a)users.noreply.github.com> date: 2024-05-31T17:27:55Z summary: [3.13] gh-100117: Fix inaccuracy in documentation of the CodeObject's co_positions field. (GH-119364) (#119869) gh-100117: Fix inaccuracy in documentation of the CodeObject's co_positions field. (GH-119364) (cherry picked from commit 015b1fdd0ae03f94a5dfda051b020810d1c952dd) Co-authored-by: Irit Katriel <1055913+iritkatriel(a)users.noreply.github.com> files: M Doc/reference/datamodel.rst diff --git a/Doc/reference/datamodel.rst b/Doc/reference/datamodel.rst index 6d2da88d13c25c..6ad62a6e62ea37 100644 --- a/Doc/reference/datamodel.rst +++ b/Doc/reference/datamodel.rst @@ -1240,7 +1240,7 @@ Methods on code objects The iterator returns :class:`tuple`\s containing the ``(start_line, end_line, start_column, end_column)``. The *i-th* tuple corresponds to the - position of the source code that compiled to the *i-th* instruction. + position of the source code that compiled to the *i-th* code unit. Column information is 0-indexed utf-8 byte offsets on the given source line.

1 0

[3.12] gh-100117: Fix inaccuracy in documentation of the CodeObject's co_positions field. (GH-119364) (#119870)
by iritkatriel May 31, 2024

May 31, 2024

https://github.com/python/cpython/commit/045be7729ef9357f53a01d7c42f742a611… commit: 045be7729ef9357f53a01d7c42f742a6111a2bc0 branch: 3.12 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: iritkatriel <1055913+iritkatriel(a)users.noreply.github.com> date: 2024-05-31T18:24:11+01:00 summary: [3.12] gh-100117: Fix inaccuracy in documentation of the CodeObject's co_positions field. (GH-119364) (#119870) gh-100117: Fix inaccuracy in documentation of the CodeObject's co_positions field. (GH-119364) (cherry picked from commit 015b1fdd0ae03f94a5dfda051b020810d1c952dd) Co-authored-by: Irit Katriel <1055913+iritkatriel(a)users.noreply.github.com> files: M Doc/reference/datamodel.rst diff --git a/Doc/reference/datamodel.rst b/Doc/reference/datamodel.rst index ac7b0f4ce5c4d6..602014deebaa7b 100644 --- a/Doc/reference/datamodel.rst +++ b/Doc/reference/datamodel.rst @@ -1231,7 +1231,7 @@ Methods on code objects The iterator returns :class:`tuple`\s containing the ``(start_line, end_line, start_column, end_column)``. The *i-th* tuple corresponds to the - position of the source code that compiled to the *i-th* instruction. + position of the source code that compiled to the *i-th* code unit. Column information is 0-indexed utf-8 byte offsets on the given source line.

1 0

doc: Add glossary entry for "free threading" (#119865)
by colesbury May 31, 2024

May 31, 2024

https://github.com/python/cpython/commit/9bc6045842ebc91ec48ab163a9e1e86442… commit: 9bc6045842ebc91ec48ab163a9e1e8644231607c branch: main author: Sam Gross <colesbury(a)gmail.com> committer: colesbury <colesbury(a)gmail.com> date: 2024-05-31T13:23:29-04:00 summary: doc: Add glossary entry for "free threading" (#119865) files: M Doc/glossary.rst diff --git a/Doc/glossary.rst b/Doc/glossary.rst index 1e5bafce861e52..ae9949bc2867c4 100644 --- a/Doc/glossary.rst +++ b/Doc/glossary.rst @@ -438,6 +438,12 @@ Glossary division. Note that ``(-11) // 4`` is ``-3`` because that is ``-2.75`` rounded *downward*. See :pep:`238`. + free threading + A threading model where multiple threads can run Python bytecode + simultaneously within the same interpreter. This is in contrast to + the :term:`global interpreter lock` which allows only one thread to + execute Python bytecode at a time. See :pep:`703`. + function A series of statements which returns some value to a caller. It can also be passed zero or more :term:`arguments <argument>` which may be used in

1 0

contextlib docs: Clean up redundant 'up' after 'cleanup' (GH-119867)
by zware May 31, 2024

May 31, 2024

https://github.com/python/cpython/commit/f3fc800d5f17b144a752a262102b750bed… commit: f3fc800d5f17b144a752a262102b750bedcdaa14 branch: main author: Zachary Ware <zach(a)python.org> committer: zware <zachary.ware(a)gmail.com> date: 2024-05-31T17:19:54Z summary: contextlib docs: Clean up redundant 'up' after 'cleanup' (GH-119867) Reported by Michael Kass on docs@ files: M Doc/library/contextlib.rst diff --git a/Doc/library/contextlib.rst b/Doc/library/contextlib.rst index 73e53aec9cbf1c..bad9da52d6a6ca 100644 --- a/Doc/library/contextlib.rst +++ b/Doc/library/contextlib.rst @@ -796,7 +796,7 @@ executing that callback:: if result: stack.pop_all() -This allows the intended cleanup up behaviour to be made explicit up front, +This allows the intended cleanup behaviour to be made explicit up front, rather than requiring a separate flag variable. If a particular application uses this pattern a lot, it can be simplified

1 0

gh-100117: Fix inaccuracy in documentation of the CodeObject's co_positions field. (#119364)
by iritkatriel May 31, 2024

May 31, 2024

https://github.com/python/cpython/commit/015b1fdd0ae03f94a5dfda051b020810d1… commit: 015b1fdd0ae03f94a5dfda051b020810d1c952dd branch: main author: Irit Katriel <1055913+iritkatriel(a)users.noreply.github.com> committer: iritkatriel <1055913+iritkatriel(a)users.noreply.github.com> date: 2024-05-31T17:09:48Z summary: gh-100117: Fix inaccuracy in documentation of the CodeObject's co_positions field. (#119364) files: M Doc/reference/datamodel.rst diff --git a/Doc/reference/datamodel.rst b/Doc/reference/datamodel.rst index 0fe9681f93f135..134385ed2f1860 100644 --- a/Doc/reference/datamodel.rst +++ b/Doc/reference/datamodel.rst @@ -1243,7 +1243,7 @@ Methods on code objects The iterator returns :class:`tuple`\s containing the ``(start_line, end_line, start_column, end_column)``. The *i-th* tuple corresponds to the - position of the source code that compiled to the *i-th* instruction. + position of the source code that compiled to the *i-th* code unit. Column information is 0-indexed utf-8 byte offsets on the given source line.

1 0

gh-119369: Fix deadlock during thread exit in free-threaded build (#119528)
by colesbury May 31, 2024

May 31, 2024

https://github.com/python/cpython/commit/078b8c8cf2bf68f7484cc4d2e3dd74b6fa… commit: 078b8c8cf2bf68f7484cc4d2e3dd74b6fab55664 branch: main author: Sam Gross <colesbury(a)gmail.com> committer: colesbury <colesbury(a)gmail.com> date: 2024-05-31T13:04:59-04:00 summary: gh-119369: Fix deadlock during thread exit in free-threaded build (#119528) Release the GIL before calling `_Py_qsbr_unregister`. The deadlock could occur when the GIL was enabled at runtime. The `_Py_qsbr_unregister` call might block while holding the GIL because the thread state was not active, but the GIL was still held. files: A Misc/NEWS.d/next/Core and Builtins/2024-05-24-21-16-52.gh-issue-119369.qBThho.rst M Python/pystate.c M Python/qsbr.c diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-05-24-21-16-52.gh-issue-119369.qBThho.rst b/Misc/NEWS.d/next/Core and Builtins/2024-05-24-21-16-52.gh-issue-119369.qBThho.rst new file mode 100644 index 00000000000000..7abdd5cd85ccd6 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-05-24-21-16-52.gh-issue-119369.qBThho.rst @@ -0,0 +1,2 @@ +Fix deadlock during thread deletion in free-threaded build, which could +occur when the GIL was enabled at runtime. diff --git a/Python/pystate.c b/Python/pystate.c index ad7e082ce0d37e..36e4206b4a282e 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1751,7 +1751,7 @@ decrement_stoptheworld_countdown(struct _stoptheworld_state *stw); /* Common code for PyThreadState_Delete() and PyThreadState_DeleteCurrent() */ static void -tstate_delete_common(PyThreadState *tstate) +tstate_delete_common(PyThreadState *tstate, int release_gil) { assert(tstate->_status.cleared && !tstate->_status.finalized); tstate_verify_not_active(tstate); @@ -1793,10 +1793,6 @@ tstate_delete_common(PyThreadState *tstate) HEAD_UNLOCK(runtime); -#ifdef Py_GIL_DISABLED - _Py_qsbr_unregister(tstate); -#endif - // XXX Unbind in PyThreadState_Clear(), or earlier // (and assert not-equal here)? if (tstate->_status.bound_gilstate) { @@ -1807,6 +1803,14 @@ tstate_delete_common(PyThreadState *tstate) // XXX Move to PyThreadState_Clear()? clear_datastack(tstate); + if (release_gil) { + _PyEval_ReleaseLock(tstate->interp, tstate, 1); + } + +#ifdef Py_GIL_DISABLED + _Py_qsbr_unregister(tstate); +#endif + tstate->_status.finalized = 1; } @@ -1818,7 +1822,7 @@ zapthreads(PyInterpreterState *interp) when the threads are all really dead (XXX famous last words). */ while ((tstate = interp->threads.head) != NULL) { tstate_verify_not_active(tstate); - tstate_delete_common(tstate); + tstate_delete_common(tstate, 0); free_threadstate((_PyThreadStateImpl *)tstate); } } @@ -1829,7 +1833,7 @@ PyThreadState_Delete(PyThreadState *tstate) { _Py_EnsureTstateNotNULL(tstate); tstate_verify_not_active(tstate); - tstate_delete_common(tstate); + tstate_delete_common(tstate, 0); free_threadstate((_PyThreadStateImpl *)tstate); } @@ -1842,8 +1846,7 @@ _PyThreadState_DeleteCurrent(PyThreadState *tstate) _Py_qsbr_detach(((_PyThreadStateImpl *)tstate)->qsbr); #endif current_fast_clear(tstate->interp->runtime); - tstate_delete_common(tstate); - _PyEval_ReleaseLock(tstate->interp, tstate, 1); + tstate_delete_common(tstate, 1); // release GIL as part of call free_threadstate((_PyThreadStateImpl *)tstate); } diff --git a/Python/qsbr.c b/Python/qsbr.c index 1e02ff9c2e45f0..9cbce9044e2941 100644 --- a/Python/qsbr.c +++ b/Python/qsbr.c @@ -236,6 +236,11 @@ _Py_qsbr_unregister(PyThreadState *tstate) struct _qsbr_shared *shared = &tstate->interp->qsbr; struct _PyThreadStateImpl *tstate_imp = (_PyThreadStateImpl*) tstate; + // gh-119369: GIL must be released (if held) to prevent deadlocks, because + // we might not have an active tstate, which means taht blocking on PyMutex + // locks will not implicitly release the GIL. + assert(!tstate->_status.holds_gil); + PyMutex_Lock(&shared->mutex); // NOTE: we must load (or reload) the thread state's qbsr inside the mutex // because the array may have been resized (changing tstate->qsbr) while

1 0

gh-119690: Adds Unicode support for named pipes in _winapi (GH-119717)
by zooba May 31, 2024

May 31, 2024

https://github.com/python/cpython/commit/8e0777df1fd7145e2e74104534645aabb6… commit: 8e0777df1fd7145e2e74104534645aabb648eea3 branch: 3.13 author: Miss Islington (bot) <31488909+miss-islington(a)users.noreply.github.com> committer: zooba <steve.dower(a)microsoft.com> date: 2024-05-31T16:22:30Z summary: gh-119690: Adds Unicode support for named pipes in _winapi (GH-119717) (cherry picked from commit 78d697b7d5ec2a6fa046b0e1c34e804f49e750b4) Co-authored-by: Steve Dower <steve.dower(a)python.org> files: A Misc/NEWS.d/next/Windows/2024-05-29-11-06-12.gh-issue-119690.8q6e1p.rst M Lib/test/audit-tests.py M Lib/test/test_audit.py M Lib/test/test_winapi.py M Modules/_winapi.c M Modules/clinic/_winapi.c.h diff --git a/Lib/test/audit-tests.py b/Lib/test/audit-tests.py index de7d0da560a1c7..b9021467817f27 100644 --- a/Lib/test/audit-tests.py +++ b/Lib/test/audit-tests.py @@ -556,6 +556,17 @@ def hook(event, args): sys.monitoring.register_callback(1, 1, None) +def test_winapi_createnamedpipe(pipe_name): + import _winapi + + def hook(event, args): + if event == "_winapi.CreateNamedPipe": + print(event, args) + + sys.addaudithook(hook) + _winapi.CreateNamedPipe(pipe_name, _winapi.PIPE_ACCESS_DUPLEX, 8, 2, 0, 0, 0, 0) + + if __name__ == "__main__": from test.support import suppress_msvcrt_asserts diff --git a/Lib/test/test_audit.py b/Lib/test/test_audit.py index e163c7ad25cc7b..321d4f9abce8c7 100644 --- a/Lib/test/test_audit.py +++ b/Lib/test/test_audit.py @@ -291,6 +291,20 @@ def test_sys_monitoring_register_callback(self): self.assertEqual(actual, expected) + def test_winapi_createnamedpipe(self): + winapi = import_helper.import_module("_winapi") + + pipe_name = r"\\.\pipe\LOCAL\test_winapi_createnamed_pipe" + returncode, events, stderr = self.run_python("test_winapi_createnamedpipe", pipe_name) + if returncode: + self.fail(stderr) + + if support.verbose: + print(*events, sep='\n') + actual = [(ev[0], ev[2]) for ev in events] + expected = [("_winapi.CreateNamedPipe", f"({pipe_name!r}, 3, 8)")] + + self.assertEqual(actual, expected) if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_winapi.py b/Lib/test/test_winapi.py index 2ac6f3621710cd..73b8228ddc97ba 100644 --- a/Lib/test/test_winapi.py +++ b/Lib/test/test_winapi.py @@ -7,7 +7,7 @@ import threading import time import unittest -from test.support import import_helper +from test.support import import_helper, os_helper _winapi = import_helper.import_module('_winapi', required_on=['win']) @@ -127,3 +127,35 @@ def test_getshortpathname(self): # Should contain "PROGRA~" but we can't predict the number self.assertIsNotNone(re.match(r".\:\\PROGRA~\d", actual.upper()), actual) + + def test_namedpipe(self): + pipe_name = rf"\\.\pipe\LOCAL\{os_helper.TESTFN}" + + # Pipe does not exist, so this raises + with self.assertRaises(FileNotFoundError): + _winapi.WaitNamedPipe(pipe_name, 0) + + pipe = _winapi.CreateNamedPipe( + pipe_name, + _winapi.PIPE_ACCESS_DUPLEX, + 8, # 8=PIPE_REJECT_REMOTE_CLIENTS + 2, # two instances available + 32, 32, 0, 0) + self.addCleanup(_winapi.CloseHandle, pipe) + + # Pipe instance is available, so this passes + _winapi.WaitNamedPipe(pipe_name, 0) + + with open(pipe_name, 'w+b') as pipe2: + # No instances available, so this times out + # (WinError 121 does not get mapped to TimeoutError) + with self.assertRaises(OSError): + _winapi.WaitNamedPipe(pipe_name, 0) + + _winapi.WriteFile(pipe, b'testdata') + self.assertEqual(b'testdata', pipe2.read(8)) + + self.assertEqual((b'', 0), _winapi.PeekNamedPipe(pipe, 8)[:2]) + pipe2.write(b'testdata') + pipe2.flush() + self.assertEqual((b'testdata', 8), _winapi.PeekNamedPipe(pipe, 8)[:2]) diff --git a/Misc/NEWS.d/next/Windows/2024-05-29-11-06-12.gh-issue-119690.8q6e1p.rst b/Misc/NEWS.d/next/Windows/2024-05-29-11-06-12.gh-issue-119690.8q6e1p.rst new file mode 100644 index 00000000000000..84dd2161aa1db8 --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2024-05-29-11-06-12.gh-issue-119690.8q6e1p.rst @@ -0,0 +1 @@ +Adds Unicode support and fixes audit events for ``_winapi.CreateNamedPipe``. diff --git a/Modules/_winapi.c b/Modules/_winapi.c index cd5dd503abe61f..8794d568e92a36 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -224,7 +224,6 @@ create_converter('LPCVOID', '" F_POINTER "') create_converter('BOOL', 'i') # F_BOOL used previously (always 'i') create_converter('DWORD', 'k') # F_DWORD is always "k" (which is much shorter) -create_converter('LPCTSTR', 's') create_converter('UINT', 'I') # F_UINT used previously (always 'I') class LPCWSTR_converter(Py_UNICODE_converter): @@ -259,7 +258,7 @@ class LPVOID_return_converter(CReturnConverter): data.return_conversion.append( 'return_value = HANDLE_TO_PYNUM(_return_value);\n') [python start generated code]*/ -/*[python end generated code: output=da39a3ee5e6b4b0d input=ef52a757a1830d92]*/ +/*[python end generated code: output=da39a3ee5e6b4b0d input=da0a4db751936ee7]*/ #include "clinic/_winapi.c.h" @@ -530,7 +529,7 @@ _winapi_CreateFile_impl(PyObject *module, LPCWSTR file_name, { HANDLE handle; - if (PySys_Audit("_winapi.CreateFile", "uIIII", + if (PySys_Audit("_winapi.CreateFile", "ukkkk", file_name, desired_access, share_mode, creation_disposition, flags_and_attributes) < 0) { return INVALID_HANDLE_VALUE; @@ -777,7 +776,7 @@ _winapi_CreateMutexW_impl(PyObject *module, /*[clinic input] _winapi.CreateNamedPipe -> HANDLE - name: LPCTSTR + name: LPCWSTR open_mode: DWORD pipe_mode: DWORD max_instances: DWORD @@ -789,25 +788,25 @@ _winapi.CreateNamedPipe -> HANDLE [clinic start generated code]*/ static HANDLE -_winapi_CreateNamedPipe_impl(PyObject *module, LPCTSTR name, DWORD open_mode, +_winapi_CreateNamedPipe_impl(PyObject *module, LPCWSTR name, DWORD open_mode, DWORD pipe_mode, DWORD max_instances, DWORD out_buffer_size, DWORD in_buffer_size, DWORD default_timeout, LPSECURITY_ATTRIBUTES security_attributes) -/*[clinic end generated code: output=80f8c07346a94fbc input=5a73530b84d8bc37]*/ +/*[clinic end generated code: output=7d6fde93227680ba input=5bd4e4a55639ee02]*/ { HANDLE handle; - if (PySys_Audit("_winapi.CreateNamedPipe", "uII", + if (PySys_Audit("_winapi.CreateNamedPipe", "ukk", name, open_mode, pipe_mode) < 0) { return INVALID_HANDLE_VALUE; } Py_BEGIN_ALLOW_THREADS - handle = CreateNamedPipe(name, open_mode, pipe_mode, - max_instances, out_buffer_size, - in_buffer_size, default_timeout, - security_attributes); + handle = CreateNamedPipeW(name, open_mode, pipe_mode, + max_instances, out_buffer_size, + in_buffer_size, default_timeout, + security_attributes); Py_END_ALLOW_THREADS if (handle == INVALID_HANDLE_VALUE) @@ -1790,7 +1789,7 @@ _winapi_OpenEventW_impl(PyObject *module, DWORD desired_access, { HANDLE handle; - if (PySys_Audit("_winapi.OpenEventW", "Iu", desired_access, name) < 0) { + if (PySys_Audit("_winapi.OpenEventW", "ku", desired_access, name) < 0) { return INVALID_HANDLE_VALUE; } @@ -1821,7 +1820,7 @@ _winapi_OpenMutexW_impl(PyObject *module, DWORD desired_access, { HANDLE handle; - if (PySys_Audit("_winapi.OpenMutexW", "Iu", desired_access, name) < 0) { + if (PySys_Audit("_winapi.OpenMutexW", "ku", desired_access, name) < 0) { return INVALID_HANDLE_VALUE; } @@ -1882,7 +1881,7 @@ _winapi_OpenProcess_impl(PyObject *module, DWORD desired_access, { HANDLE handle; - if (PySys_Audit("_winapi.OpenProcess", "II", + if (PySys_Audit("_winapi.OpenProcess", "kk", process_id, desired_access) < 0) { return INVALID_HANDLE_VALUE; } @@ -2236,19 +2235,19 @@ _winapi_VirtualQuerySize_impl(PyObject *module, LPCVOID address) /*[clinic input] _winapi.WaitNamedPipe - name: LPCTSTR + name: LPCWSTR timeout: DWORD / [clinic start generated code]*/ static PyObject * -_winapi_WaitNamedPipe_impl(PyObject *module, LPCTSTR name, DWORD timeout) -/*[clinic end generated code: output=c2866f4439b1fe38 input=36fc781291b1862c]*/ +_winapi_WaitNamedPipe_impl(PyObject *module, LPCWSTR name, DWORD timeout) +/*[clinic end generated code: output=e161e2e630b3e9c2 input=099a4746544488fa]*/ { BOOL success; Py_BEGIN_ALLOW_THREADS - success = WaitNamedPipe(name, timeout); + success = WaitNamedPipeW(name, timeout); Py_END_ALLOW_THREADS if (!success) @@ -2917,7 +2916,7 @@ _winapi_CopyFile2_impl(PyObject *module, LPCWSTR existing_file_name, HRESULT hr; COPYFILE2_EXTENDED_PARAMETERS params = { sizeof(COPYFILE2_EXTENDED_PARAMETERS) }; - if (PySys_Audit("_winapi.CopyFile2", "uuI", + if (PySys_Audit("_winapi.CopyFile2", "uuk", existing_file_name, new_file_name, flags) < 0) { return NULL; } diff --git a/Modules/clinic/_winapi.c.h b/Modules/clinic/_winapi.c.h index 9acb2dc4fe7eba..b0c54fc809f4c1 100644 --- a/Modules/clinic/_winapi.c.h +++ b/Modules/clinic/_winapi.c.h @@ -445,7 +445,7 @@ PyDoc_STRVAR(_winapi_CreateNamedPipe__doc__, {"CreateNamedPipe", _PyCFunction_CAST(_winapi_CreateNamedPipe), METH_FASTCALL, _winapi_CreateNamedPipe__doc__}, static HANDLE -_winapi_CreateNamedPipe_impl(PyObject *module, LPCTSTR name, DWORD open_mode, +_winapi_CreateNamedPipe_impl(PyObject *module, LPCWSTR name, DWORD open_mode, DWORD pipe_mode, DWORD max_instances, DWORD out_buffer_size, DWORD in_buffer_size, DWORD default_timeout, @@ -455,7 +455,7 @@ static PyObject * _winapi_CreateNamedPipe(PyObject *module, PyObject *const *args, Py_ssize_t nargs) { PyObject *return_value = NULL; - LPCTSTR name; + LPCWSTR name = NULL; DWORD open_mode; DWORD pipe_mode; DWORD max_instances; @@ -465,8 +465,8 @@ _winapi_CreateNamedPipe(PyObject *module, PyObject *const *args, Py_ssize_t narg LPSECURITY_ATTRIBUTES security_attributes; HANDLE _return_value; - if (!_PyArg_ParseStack(args, nargs, "skkkkkk" F_POINTER ":CreateNamedPipe", - &name, &open_mode, &pipe_mode, &max_instances, &out_buffer_size, &in_buffer_size, &default_timeout, &security_attributes)) { + if (!_PyArg_ParseStack(args, nargs, "O&kkkkkk" F_POINTER ":CreateNamedPipe", + _PyUnicode_WideCharString_Converter, &name, &open_mode, &pipe_mode, &max_instances, &out_buffer_size, &in_buffer_size, &default_timeout, &security_attributes)) { goto exit; } _return_value = _winapi_CreateNamedPipe_impl(module, name, open_mode, pipe_mode, max_instances, out_buffer_size, in_buffer_size, default_timeout, security_attributes); @@ -479,6 +479,9 @@ _winapi_CreateNamedPipe(PyObject *module, PyObject *const *args, Py_ssize_t narg return_value = HANDLE_TO_PYNUM(_return_value); exit: + /* Cleanup for name */ + PyMem_Free((void *)name); + return return_value; } @@ -1660,22 +1663,25 @@ PyDoc_STRVAR(_winapi_WaitNamedPipe__doc__, {"WaitNamedPipe", _PyCFunction_CAST(_winapi_WaitNamedPipe), METH_FASTCALL, _winapi_WaitNamedPipe__doc__}, static PyObject * -_winapi_WaitNamedPipe_impl(PyObject *module, LPCTSTR name, DWORD timeout); +_winapi_WaitNamedPipe_impl(PyObject *module, LPCWSTR name, DWORD timeout); static PyObject * _winapi_WaitNamedPipe(PyObject *module, PyObject *const *args, Py_ssize_t nargs) { PyObject *return_value = NULL; - LPCTSTR name; + LPCWSTR name = NULL; DWORD timeout; - if (!_PyArg_ParseStack(args, nargs, "sk:WaitNamedPipe", - &name, &timeout)) { + if (!_PyArg_ParseStack(args, nargs, "O&k:WaitNamedPipe", + _PyUnicode_WideCharString_Converter, &name, &timeout)) { goto exit; } return_value = _winapi_WaitNamedPipe_impl(module, name, timeout); exit: + /* Cleanup for name */ + PyMem_Free((void *)name); + return return_value; } @@ -2118,4 +2124,4 @@ _winapi_CopyFile2(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyO return return_value; } -/*[clinic end generated code: output=ed94a2482ede3744 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=2304c62187a90140 input=a9049054013a1b77]*/

1 0

[3.12] gh-119585: Fix crash involving `PyGILState_Release()` and `PyThreadState_Clear()` (GH-119753) (#119861)
by colesbury May 31, 2024

May 31, 2024

https://github.com/python/cpython/commit/738cf216093e844c154f9ba36605f485fa… commit: 738cf216093e844c154f9ba36605f485fac5ff0d branch: 3.12 author: Sam Gross <colesbury(a)gmail.com> committer: colesbury <colesbury(a)gmail.com> date: 2024-05-31T15:42:09Z summary: [3.12] gh-119585: Fix crash involving `PyGILState_Release()` and `PyThreadState_Clear()` (GH-119753) (#119861) Make sure that `gilstate_counter` is not zero in when calling `PyThreadState_Clear()`. A destructor called from `PyThreadState_Clear()` may call back into `PyGILState_Ensure()` and `PyGILState_Release()`. If `gilstate_counter` is zero, it will try to create a new thread state before the current active thread state is destroyed, leading to an assertion failure or crash. (cherry picked from commit bcc1be39cb1d04ad9fc0bd1b9193d3972835a57c) files: A Misc/NEWS.d/next/C API/2024-05-29-21-05-59.gh-issue-119585.Sn7JL3.rst M Lib/test/test_capi/test_misc.py M Modules/_testcapimodule.c M Python/pystate.c diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 37dff355b1ca64..98c74a44e4cb27 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -2094,6 +2094,22 @@ def callback(): t.start() t.join() + @threading_helper.reap_threads + @threading_helper.requires_working_threading() + def test_thread_gilstate_in_clear(self): + # See https://github.com/python/cpython/issues/119585 + class C: + def __del__(self): + _testcapi.gilstate_ensure_release() + + # Thread-local variables are destroyed in `PyThreadState_Clear()`. + local_var = threading.local() + + def callback(): + local_var.x = C() + + _testcapi._test_thread_state(callback) + @threading_helper.reap_threads @threading_helper.requires_working_threading() def test_gilstate_ensure_no_deadlock(self): diff --git a/Misc/NEWS.d/next/C API/2024-05-29-21-05-59.gh-issue-119585.Sn7JL3.rst b/Misc/NEWS.d/next/C API/2024-05-29-21-05-59.gh-issue-119585.Sn7JL3.rst new file mode 100644 index 00000000000000..038dec2dbf90d1 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2024-05-29-21-05-59.gh-issue-119585.Sn7JL3.rst @@ -0,0 +1,5 @@ +Fix crash when a thread state that was created by :c:func:`PyGILState_Ensure` +calls a destructor that during :c:func:`PyThreadState_Clear` that +calls back into :c:func:`PyGILState_Ensure` and :c:func:`PyGILState_Release`. +This might occur when in the free-threaded build or when using thread-local +variables whose destructors call :c:func:`PyGILState_Ensure`. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index edd137f30ca9ba..aece635554daa0 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -822,6 +822,14 @@ test_thread_state(PyObject *self, PyObject *args) Py_RETURN_NONE; } +static PyObject * +gilstate_ensure_release(PyObject *module, PyObject *Py_UNUSED(ignored)) +{ + PyGILState_STATE state = PyGILState_Ensure(); + PyGILState_Release(state); + Py_RETURN_NONE; +} + #ifndef MS_WINDOWS static PyThread_type_lock wait_done = NULL; @@ -3267,6 +3275,7 @@ static PyMethodDef TestMethods[] = { {"test_get_type_qualname", test_get_type_qualname, METH_NOARGS}, {"test_get_type_dict", test_get_type_dict, METH_NOARGS}, {"_test_thread_state", test_thread_state, METH_VARARGS}, + {"gilstate_ensure_release", gilstate_ensure_release, METH_NOARGS}, #ifndef MS_WINDOWS {"_spawn_pthread_waiter", spawn_pthread_waiter, METH_NOARGS}, {"_end_spawned_pthread", end_spawned_pthread, METH_NOARGS}, diff --git a/Python/pystate.c b/Python/pystate.c index 1337516aa59cbc..d0651fbd592f43 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2277,12 +2277,18 @@ PyGILState_Release(PyGILState_STATE oldstate) /* can't have been locked when we created it */ assert(oldstate == PyGILState_UNLOCKED); // XXX Unbind tstate here. + // gh-119585: `PyThreadState_Clear()` may call destructors that + // themselves use PyGILState_Ensure and PyGILState_Release, so make + // sure that gilstate_counter is not zero when calling it. + ++tstate->gilstate_counter; PyThreadState_Clear(tstate); + --tstate->gilstate_counter; /* Delete the thread-state. Note this releases the GIL too! * It's vital that the GIL be held here, to avoid shutdown * races; see bugs 225673 and 1061968 (that nasty bug has a * habit of coming back). */ + assert(tstate->gilstate_counter == 0); assert(current_fast_get(runtime) == tstate); _PyThreadState_DeleteCurrent(tstate); }

1 0