memoryview can set an exception in tp_clear

@pitrou

BPO	33713
Nosy	@pitrou, @vstinner, @skrah, @serhiy-storchaka

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2018-05-31.12:15:10.896>
labels = ['interpreter-core', '3.10', '3.9', 'type-crash', '3.11']
title = 'memoryview can set an exception in tp_clear'
updated_at = <Date 2021-10-18.20:19:58.191>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2021-10-18.20:19:58.191>
actor = 'iritkatriel'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2018-05-31.12:15:10.896>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 33713
keywords = []
message_count = 14.0
messages = ['318288', '318291', '318295', '318296', '318297', '318298', '318299', '318300', '318306', '318316', '318318', '318319', '318320', '318586']
nosy_count = 4.0
nosy_names = ['pitrou', 'vstinner', 'skrah', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue33713'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

Linked PRs

The tp_clear handler of memoryview can set an exception when fail to release the buffer. An exception in tp_clear is not expected and caused a crash in the garbage collector. In the master branch it will cause just writing a traceback to stderr (see bpo-33622), but in any case it would be better to handle the failure locally in memoryview. I don't know what is the best solution: silencing an error, writing a traceback with more detailed information, or resurrecting the buffer object.

Could you please show how tp_clear() can be called when self->exports > 0? It should not happen.

bpo-33622 is a big issue with many commits. Would it be possible to extract the relevant part?

I don't know how to reproduce a failure in tp_clear(). I just can't prove that it never fails. Maybe it is needed a bug in the implementation of the buffer protocol in third-party extension.

If it should not happen then we can just add

assert(!PyErr_Occurred());

or

    if (PyErr_Occurred()) {
        PyErr_WriteUnraisable(NULL);
    }

It is better to crash in memoryview.c than in the garbage collector if this crash is caused by incorrect buffer protocol implementation.

This looks the same as bpo-25525. I think it cannot happen, and no one has ever reported an actual issue for 6 years now.

You *really* need to show a reproducer if you assert that something can crash.

See the delete_garbage() function line 770 in Modules/gcmodule.c for changes in the master branch relevant to this issue. See Py_FatalError() in the collect() function at line 974 for a crash.

Yes, but who calls tp_clear() if the memoryview is not being deallocated?

The GC calls tp_clear() if the memoryview is a part of the reference loop.

a = [memoryview(...)]
a.append(a)
del a

The GC will call tp_clear() of the list or the memoryview. What will be called first is not specified.

The point is that *no garbage collection is triggered* if self->exports > 0.

It would be a major bug if it were and I suspect it would be reported within a week. Fortunately, no such bug has been reported in 6 years.

Serhiy is right about the theoretical concern here. However, it's probably quite difficult to find a concrete situation where this occurs, because we're talking about mbuf_clear and the managerbuffer object can't really get involved in a reference cycle by itself (not in normal use anyway): the memoryview object does, but it's a different thing.

By the way: PyBuffer_Release() returns void, so IMO it's a bug if it can return with an exception set. We should fix that rather than focus on mbuf_clear().

Well, the example would need exports:

>>> a = [bytes()]
>>> a.append(memoryview(a[0]))
>>> a.append(memoryview(a[1]))
>>> a.append(a)
>>> a
[b'', <memory at 0x1ad21f8>, <memory at 0x1b52348>, [...]]

The first memoryview has one export, so its refcount > 0.

Do I fundamentally misunderstand tp_clear() and tp_clear() can be called on objects with refcount > 0?

If the bug cannot occur, just add "assert(!PyErr_Occurred());" no?

gh-77894: Fix a crash when the GC breaks a loop containing a memoryvi…

pythongh-77894: Fix a crash when the GC breaks a loop containing a me…

[3.12] pythongh-77894: Fix a crash when the GC breaks a loop containi…

[3.12] gh-77894: Fix a crash when the GC breaks a loop containing a m…

[3.13] gh-77894: Fix a crash when the GC breaks a loop containing a m…

@serhiy-storchaka

@serhiy-storchaka If you are still looking for reproducible test cases, check out https://youtrack.jetbrains.com/issue/PY-54447. PyCharm users running under the debugger with code using multiprocessing and concurrent.futures have been seeing this since Python 3.10. Example code to reproduce is included.

@tjnd89

I forgot to close this issue after merging #123898.

@tjnd89, thank you for reference. Did this change fix that issue? If no, it is perhaps not related.

@serhiy-storchaka

@serhiy-storchaka I can confirm that the change seems to have fixed the issue. I ran the test code in the linked isue in Python 3.10 and reproduced the bug, then again in 3.12.7 with your fix and was not able to reproduce it. Thanks for chasing this down!

It looks like all reproducers involve some kind of getbuffer() methods that expose a memoryview on the Python level.

The previous collection scheme apparently relied on not adding these methods, which is why #58006 has not been implemented for a long time.

The new scheme apparently swallows an exception:

class B(bytearray):
    def __buffer__(self, flags):
        return super().__buffer__(flags)
    def __release_buffer__(self, view):
        super().__release_buffer__(view)

X = B(b"xyz")
X.b = memoryview(X)
X.c = memoryview(X.b)

$ ./python swallow.py 
Exception ignored in __release_buffer__ of B:
AttributeError: __release_buffer__
SystemError: deallocated bytearray object has exported buffers

The ignored exception comes from:

static void
bytearray_dealloc(PyByteArrayObject *self)
{
    if (self->ob_exports > 0) {
        PyErr_SetString(PyExc_SystemError,
                        "deallocated bytearray object has exported buffers");
        PyErr_Print();
    }
    if (self->ob_bytes != 0) {
        PyObject_Free(self->ob_bytes);
    }
    Py_TYPE(self)->tp_free((PyObject *)self);
}

I have never seen this message before all getbuffer() methods were added, which I believe started in 3.10. This would be consistent with the Jetbrains issue (linked above in this issue), where people say 3.9 worked. I'm not sure why a bytearray with exports is allowed to be deallocated.

Release cffi buffer exports explicitly to avoid a GC segfault with me…

Navigation Menu

Repository navigation

memoryview can set an exception in tp_clear #77894

Linked PRs

17 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

memoryview can set an exception in tp_clear #77894

Description

Linked PRs

Activity

serhiy-storchaka commented on May 31, 2018

skrah commented on May 31, 2018

serhiy-storchaka commented on May 31, 2018

skrah commented on May 31, 2018

serhiy-storchaka commented on May 31, 2018

skrah commented on May 31, 2018

serhiy-storchaka commented on May 31, 2018

skrah commented on May 31, 2018

pitrou commented on May 31, 2018

skrah commented on May 31, 2018

vstinner commented on May 31, 2018

17 remaining items

tjnd89 commented on Jan 14, 2025

serhiy-storchaka commented on Jan 14, 2025

tjnd89 commented on Jan 14, 2025

lisa-gll commented on May 11, 2025

lisa-gll commented on May 11, 2025

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions