gh-146536: docs: Clarify that PyMutex is not re-entrant#146543
gh-146536: docs: Clarify that PyMutex is not re-entrant#146543dagangtj wants to merge 1 commit intopython:mainfrom
Conversation
Add explicit documentation that PyMutex is not re-entrant (recursive): - Add note to PyMutex type explaining it will deadlock on re-entry - Add warning to PyMutex_Lock about deadlock if already held - Clarify PyMutex_Unlock must be called by the locking thread Fixes python#146536
bedevere-app
Bot
added
docs
StanFromIreland
changed the title
|
|
||
| .. c:function:: void PyMutex_Lock(PyMutex *m) | ||
|
|
||
| Lock mutex *m*. If another thread has already locked it, the calling |
There was a problem hiding this comment.
I guess this can be improved too:
| Lock mutex *m*. If the mutex is already locked, the calling |
| otherwise, the function will issue a fatal error. Unlocking a mutex from | ||
| a different thread than the one that locked it results in undefined | ||
| behavior. |
There was a problem hiding this comment.
Is it a guaranteed fatal error, or undefined behaviour? It can't be both at once.
| :c:type:`!PyMutex` is not re-entrant. For re-entrant locking, use | ||
| :c:func:`Py_BEGIN_CRITICAL_SECTION` instead. |
There was a problem hiding this comment.
This is redundant.
| :c:type:`!PyMutex` is not re-entrant. For re-entrant locking, use | |
| :c:func:`Py_BEGIN_CRITICAL_SECTION` instead. | |
| For re-entrant locking, use | |
| :c:func:`Py_BEGIN_CRITICAL_SECTION` instead. |
| .. note:: | ||
|
|
||
| A :c:type:`!PyMutex` is **not** re-entrant (recursive). If a thread | ||
| that already holds the lock attempts to acquire it again, the thread | ||
| will deadlock. Use :c:func:`Py_BEGIN_CRITICAL_SECTION` instead if | ||
| re-entrancy is required. | ||
|
|
There was a problem hiding this comment.
I don't think we need the note both here and on PyMutex_Lock.
| .. note:: | |
| A :c:type:`!PyMutex` is **not** re-entrant (recursive). If a thread | |
| that already holds the lock attempts to acquire it again, the thread | |
| will deadlock. Use :c:func:`Py_BEGIN_CRITICAL_SECTION` instead if | |
| re-entrancy is required. |
|
I've addressed all the review comments from @encukou:
The changes are available in patch form at the latest commit. Please review and let me know if any further adjustments are needed. Thank you for the thorough review! |
2 participants
Summary
This PR clarifies the documentation for :c:type:
PyMutexto explicitly state that it is not re-entrant (recursive).Changes
PyMutextype documentation explaining that it will deadlock if a thread attempts to acquire a lock it already holdsPyMutex_Lockabout deadlock behavior on re-entryPyMutex_Unlockmust be called by the same thread that acquired the lockBackground
The original issue reported confusion about whether PyMutex supports re-entrant locking. Since PyMutex occupies only one byte and uses atomic operations without tracking thread ownership, it cannot support re-entrancy. This documentation update makes this limitation explicit and suggests using :c:func:
Py_BEGIN_CRITICAL_SECTIONas an alternative for re-entrant locking scenarios.Fixes #146536
📚 Documentation preview 📚: https://cpython-previews--146543.org.readthedocs.build/