Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
sobolevn opened this issue Feb 18, 2023 · 4 comments
Closed

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

sobolevn opened this issue Feb 18, 2023 · 4 comments
Assignees
Labels
3.13 bugs and security fixes extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Feb 18, 2023

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

See:

  1. def __init__(self):
  2. 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

@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir extension-modules C modules in the Modules dir 3.12 bugs and security fixes labels Feb 18, 2023
@sobolevn sobolevn self-assigned this Feb 18, 2023
@sobolevn
Copy link
Member Author

sobolevn commented Feb 18, 2023

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? :)

@sobolevn
Copy link
Member Author

CC @njsmith @pitrou

@gpshead
Copy link
Member

gpshead commented May 31, 2023

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.

@sobolevn sobolevn added 3.13 bugs and security fixes and removed 3.12 bugs and security fixes labels Jun 1, 2023
@hugovk
Copy link
Member

hugovk commented Nov 10, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants