FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-80384: Check that callback is callable at weak reference creation by serhiy-storchaka · Pull Request #151145 · python/cpython · GitHub

/ cpython Public

gh-80384: Check that callback is callable at weak reference creation - #151145

Merged
serhiy-storchaka merged 2 commits into
python:mainfrom
serhiy-storchaka:weakref-check-callback-is-callable
Jun 10, 2026
Merged

gh-80384: Check that callback is callable at weak reference creation#151145
serhiy-storchaka merged 2 commits into
python:mainfrom
serhiy-storchaka:weakref-check-callback-is-callable

Conversation

serhiy-storchaka commented Jun 9, 2026
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member
  • Python functions weakref.ref() and weakref.proxy() now raise TypeError if the callback argument is not callable or None.
  • C functions PyWeakref_NewRef() and PyWeakref_NewProxy() now raise TypeError if the callback argument is not callable, None, or NULL.

read-the-docs-community Bot commented Jun 9, 2026
edited
Loading

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #33058103 | 📁 Comparing 88afc02 against main (9fdbade)

  🔍 Preview build  

3 files changed
± c-api/weakref.html
± library/weakref.html
± whatsnew/changelog.html

…ation

* Python functions weakref.ref() and weakref.proxy() now raise TypeError
  if the callback argument is not callable or None.
* C functions PyWeakref_NewRef() and PyWeakref_NewProxy() now raise TypeError
  if the callback argument is not callable, None, or NULL.
serhiy-storchaka force-pushed the weakref-check-callback-is-callable branch from 0fe7893 to 06145a6 Compare June 9, 2026 12:03

vstinner commented Jun 9, 2026

Copy link
Copy Markdown
Member

test_traceback.test_recursion_error_during_traceback() does fail. The script calls ref(lambda: 0, []) whereas [] is not callable.

Copy link
Copy Markdown
Member Author

@isidentical, do you remember the purpose of ref(lambda: 0, []) in that test (GH-27313)?

It's effect was raising a TypeError after destruction the weak-referred object (lambda: 0). Since this is not associated with any Python code, source_line should be NULL. I replaced the callback with ord, calling which should have the same effect.

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM. I just have a suggestion for test_traceback.

Comment on lines 208 to 210
def f():
ref(lambda: 0, [])
ref(lambda: 0, ord)
f()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The test expects that calling the callback raises an exception. Currently, it raises a TypeError since [] (list) is not callable. I suggest making the exception more explicit:

                def callback(wr):
                    raise Exception("error")

                def f():
                    ref(lambda: 0, callback)
                    f()

Passing ord() is more surprising to me.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If I correctly understand the purpose of this test, we cannot do this. This would insert a Python frame, this would give a non-NULL source_line. This is why the test used such indirect way of raising an exception.

I am not completely sure in my reconstruction, this is why I have not added an explaining comment, which would be helpful here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The test was added by commit c836231.

At first ref(lambda: 0, []) calls, display_source_line() returns rc=0, no exception set, source_line is not NULL. But when we are getting closer to the recursion limit, display_source_line() can fail with RecursionError on the call:

    fob = _PyObject_CallMethod(io, &_Py_ID(TextIOWrapper),
                               "Os", binary, encoding);

In this case, display_source_line() clears the exception, returns 0 and source_line is NULL. Calling ignore_source_errors() here just sets err=0 (no exception is set), but err is already equal to 0. So I don't understand well what we are testing here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I am not confident in my explanation, and adding incorrect comment may be worse than missing comment.

Copy link
Copy Markdown
Member

I'm fine with ref(lambda: 0, ord). Maybe just add a comment explaining the purpose of the test_traceback test.

serhiy-storchaka merged commit ca32ebf into python:main Jun 10, 2026
59 checks passed
serhiy-storchaka deleted the weakref-check-callback-is-callable branch June 10, 2026 10:35

Copy link
Copy Markdown
Member Author

I added @tekknolagi as a co-author. Even if this PR wasn't technically based on #26273, he started in the right place, and it's not his fault that his PR didn't get enough attention at the time.

philthompson10 pushed a commit to philthompson10/cpython that referenced this pull request Jun 17, 2026
…ation (pythonGH-151145)

* Python functions weakref.ref() and weakref.proxy() now raise TypeError
  if the callback argument is not callable or None.
* C functions PyWeakref_NewRef() and PyWeakref_NewProxy() now raise TypeError
  if the callback argument is not callable, None, or NULL.

Co-authored-by: Maxwell Bernstein <emacs@fb.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL