gh-104223: Fix issues with inheriting from buffer classes

[JelleZijlstra]

This fixes various issues that crop up when inheriting from a class that implements the C buffer protocol.

• In wrap_buffer, we need to make sure to call the exact slot that we're wrapping, or we might recursively call ourselves inside a call to super().
• In bufferwrapper_releasebuf, we should only call the bf_releasebuf slot if it's a Python function. If it's a C slot inherited from a base class, it will just be called when we release the memoryview.
• The C bf_releasebuf slot of classes that inherit from C buffer classes was not called. Fix this.
• The wrapper memoryview created in releasebuffer_call_python allowed a use-after-free. To guard against that, create a new restricted-mode memoryview that does not allow the user to hold on to the buffer.

• Issue: PEP 688: Cannot call super().__buffer__() #104223

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @JelleZijlstra for commit 88ce24d 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

[chilaxan]

It looks like when you export a python class that subclasses an internal class that implements __buffer__, the memoryview produced is not properly tracked. this means that if the python class has a __release_buffer__ that stores the passed in buffer it will not respect the state of the exported object and vise-versa:

>>> class B(bytearray):
...     def __release_buffer__(self, buffer):
...             B.leak = self.clear() or bytearray()
...             B.backing = buffer
... 
>>> b = B(bytearray.__basicsize__)
>>> m = memoryview(b)
>>> m.release()
>>> B.leak
bytearray(b'')
>>> B.backing
<memory at 0x109d1f700>
>>> B.backing.cast('P').tolist()
[1, 4454897440, 0, 0, 0, 0, 0]
>>> B.backing.cast('P')[2] = -1
>>> len(B.leak)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
SystemError: <built-in function len> returned NULL without setting an exception

[kumaraditya303]

Some high level comments, mostly looks good.

[kumaraditya303]

Can you add a test in which the class has multiple inheritance? Also tests for when there are two or classes in mro which support buffer protocol?

[JelleZijlstra]

Adding some tests. It's hard to come up with a case where multiple bases support the C buffer protocol, because that will almost inevitably lead to:

Traceback (most recent call last):
  File "/Users/jelle/py/cpython/Lib/test/test_buffer.py", line 4707, in test_two_buffer_bases
    class A(bytearray, bytes):
TypeError: multiple bases have instance lay-out conflict

[kumaraditya303]

Hmm, maybe try one var length and one pure python type which implements buffer protocol.

[JelleZijlstra]

Yes, just pushed. Thanks for the review!

[kumaraditya303]

Adding some tests. It's hard to come up with a case where multiple bases support the C buffer protocol, because that will almost inevitably lead to:

I see, you would have to write a custom type in C whose layout doesn't conflicts for that. Probably an object which has just object header and supports buffer protocol.

[kumaraditya303]

TBH, I agree this is an edge case but I try to be extra careful when touching typeobject, hope you don't mind the extra work.

[JelleZijlstra]

Better to catch this now then right before 3.12 final!

[JelleZijlstra]

Confirmed the bug I outlined above, I'll have to step out for a few hours and fix it after that.

[JelleZijlstra]

Thinking about it more, that crash isn't actually due to PEP 688: it reproduces without any Python __buffer__ method. So I don't think there's any more interesting cases to cover for PEP 688.

[JelleZijlstra]

Filed #104297 for that case.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @JelleZijlstra for commit 5536853 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.