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

[serhiy-storchaka]

• 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.

• Issue: PyWeakref_NewRef docs are misleading #80384

[read-the-docs-community]

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

[vstinner]

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

[serhiy-storchaka]

@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]

LGTM. I just have a suggestion for test_traceback.

[vstinner]

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.

[serhiy-storchaka]

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.

[vstinner]

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.

[serhiy-storchaka]

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

[vstinner]

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

[serhiy-storchaka]

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.