Skip to content

threading.RLock must not support *args, **kwargs arguments #102029

Closed
Closed
threading.RLock must not support *args, **kwargs arguments#102029
Assignees
sobolevn
Labels
3.13only security fixesextension-modulesC modules in the Modules dirstdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

Description

@sobolevn
sobolevn
opened on Feb 18, 2023

Right now we have an interesting problem: _CRLock supports *args, **kwargs in __new__, while _PYRLock does not.

See:

  1. cpython/Lib/threading.py

    Line 133 in 128379b

    def __init__(self):

  2. cpython/Modules/_threadmodule.c

    Line 506 in 128379b

    rlock_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
    (they are unused)

I am leaving aside the problem that _PYRLock is not ever used (unless imported directly) for now.

Right now our docs does not say what the signature should be.
Some facts:

  1. CPython's code never use RLock with arguments (and never say it has arguments)
  2. typeshed (a source of truth for all typecheckers and several IDEs never had a single issue about missing *args, **kwargs in their RLock's stub: https://github.com/python/typeshed/blob/0bb7d621d39d38bee7ce32e1ee920bd5bc4f9503/stdlib/threading.pyi#L113-L119
  3. We don't have any docs about RLock's signature

So, I think we can:

  1. Document that RLock has () signature
  2. Remove *args, **kwargs from C impl
  3. Document this in "What's new"

Does it sound like a plan? If yes, I would like to do the honours :)

Linked PRs

Activity

  1. sobolevn
    added
    type-bugAn unexpected behavior, bug, or error
    stdlibStandard Library Python modules in the Lib/ directory
    extension-modulesC modules in the Modules dir
    3.12only security fixes
    on Feb 18, 2023
  2. sobolevn
    self-assigned this
    on Feb 18, 2023

  3. sobolevn commented on Feb 18, 2023

    @sobolevn
    sobolevn
    on Feb 18, 2023
    MemberAuthor

    Or even it migth be a good time to also deprecate _PyRLock completely?

    See #57906
    This was discussed even 11 years ago. Maybe the time has come? :)

  4. sobolevn commented on Feb 19, 2023

    @sobolevn
    sobolevn
    on Feb 19, 2023
    MemberAuthor

    CC @njsmith @pitrou

  5. sobolevn
    added a commit that references this issue on Feb 20, 2023

    pythongh-102029: Sync `__new__` sigs of `_CRLock` and `_PyRLock` in `…

    4529493

  6. gpshead commented on May 31, 2023

    @gpshead
    gpshead
    on May 31, 2023
    Member

    History:

    Python up through 2.7 and 3.1 threading.RLock supported an undocumented verbose argument. https://github.com/python/cpython/blob/v2.7.18/Lib/threading.py#L132

    It was still explicitly allowed but pulled out from the args in 3.2 even though it was not passed on to the new _CRLock.

    There is a very tiny amount of code out there that still blindly passes a value for this (now no-op) argument. The fix to such code is trivial: remove the no-op argument.

  7. sobolevn
    added
    3.13only security fixes
    and removed
    3.12only security fixes
    on Jun 1, 2023
  8. gpshead
    added a commit that references this issue on Aug 17, 2023

    gh-102029: Deprecate passing arguments to `_PyRLock` in `threading` (#…

    80f30cf

  9. hugovk commented on Nov 10, 2023

    @hugovk
    hugovk
    on Nov 10, 2023
    Member

    Anything more to do here, or can we close this issue?

  10. sobolevn
    closed this as completedon Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

3.13only security fixesextension-modulesC modules in the Modules dirstdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions