[issue15821] PyMemoryView_FromBuffer() behavior change (possible regression)
Changes by Jesús Cea Avión j...@jcea.es: -- nosy: +jcea ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15821 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15821] PyMemoryView_FromBuffer() behavior change (possible regression)
Stefan Krah added the comment:
In general, the number of calls to PyObject_GetBuffer() must be equal to
the number of calls to PyBuffer_Release(), otherwise bad things will happen
in the same way as with malloc()/free().
Now, in my test case I omitted the call to PyObject_GetBuffer() *with*
the internal knowledge that in the case of the bytes object it does
not matter: The releasebufferproc is NULL, so PyBuffer_Release()
is just a way of decrementing the reference count of the bytes object.
No matter how you turn it, if info.obj != NULL you need some internal
knowledge what the obj's releasebufferproc will do. For example, I
suspect that many existing releasebufferprocs do not account for
the fact that a consumer may change shape, strides and suboffsets.
So you need to know the exporter really well.
My strategy here is this: I want to keep backwards compatibility
for PyMemoryView_FromBuffer(), which I think should be deprecated
at some point.
A proper function that returns a memoryview with automatic cleanup
would not involve getbuffer/releasebuffer at all and could look like
this:
Flags:
--
PyManagedBuffer_FreeBuf
PyManagedBuffer_FreeFormat
PyManagedBuffer_FreeArrays // shape, strides, suboffsets
PyMemoryView_FromBufferWithCleanup(info, flags)
{
/* info should never have been obtained by a call to PyObject_GetBuffer().
Hence it should never be released. */
assert(info.obj == NULL);
/* buf, format, shape, strides and suboffsets MUST stay valid
for the lifetime of the returned memoryview. Usually they
will be dynamically allocated using PyMem_Malloc(). This is
no problem, since there is automatic cleanup in mbuf_dealloc(). */
mbuf = mbuf_alloc();
mbuf-master = *info;
mbuf-flags |= flags;
...
return new memoryview
}
mbuf_dealloc(self)
{
if (self-flagsPyManagedBuffer_FreeBuf)
PyMem_Free(self-master.buf);
...
}
This is much more flexible than the existing function and does not suffer
from any abuse of getbuffer/releasebuffer.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15821
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15821] PyMemoryView_FromBuffer() behavior change (possible regression)
Stefan Krah added the comment: Alexander, your test case is brilliant and could eventually go into _testbuffer.c, but I'd prefer a simpler test case for now. :) Here's a patch that restores the info.obj cleanup facility. I intentionally did not add copying the format in order to keep the diff small (in case this can go into rc2). Crashing on stack addresses for format is not a regression. Apparently PyMemoryView_FromBuffer() is used for low level tinkering and automatic cleanup, so I now think that it's better not to break backwards compatibility, i.e. leave the value of info.obj untouched. Otherwise people using the cleanup facility will have reference leaks. However, I don't like the interface at all: Passing a pointer to a struct and then relying on the fact that *some* values are copied (shape, strides and suboffsets) but not others (format, buf) is counterintuitive. With the ManagedBuffer, we could now write a new function that returns a memoryview with much nicer cleanup facilities. But that doesn't help here since we're stuck with this function for 3.3.0. What do you think? Should this go into 3.3.0? The changes to memoryobject.c are minimal, the rest are tests and docs. -- assignee: docs@python - skrah components: +Interpreter Core -Documentation stage: - patch review title: Improve docs for PyMemoryView_FromBuffer() - PyMemoryView_FromBuffer() behavior change (possible regression) type: enhancement - behavior Added file: http://bugs.python.org/file27076/issue15821.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15821 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15821] PyMemoryView_FromBuffer() behavior change (possible regression)
Alexander Belopolsky added the comment: On Fri, Aug 31, 2012 at 2:34 PM, Stefan Krah rep...@bugs.python.org wrote: With the ManagedBuffer, we could now write a new function that returns a memoryview with much nicer cleanup facilities. But that doesn't help here since we're stuck with this function for 3.3.0. What do you think? Should this go into 3.3.0? I am still getting up to speed with all the changes that went in since 3.2. I'll review your patch over the weekend. Meanwhile, I think the goal should be that after PyMemoryview_FromBuffer(info) is called, it should be OK to discard info by calling PyBuffer_Release() and if info is not allocated on the stack, Py_DECREF(info). I think we are almost there achieving that goal with possible exception of dynamically or stack-allocated fmt strings. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15821 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15821] PyMemoryView_FromBuffer() behavior change (possible regression)
Stefan Krah added the comment: Alexander Belopolsky rep...@bugs.python.org wrote: I am still getting up to speed with all the changes that went in since 3.2. I'll review your patch over the weekend. Meanwhile, I think the goal should be that after PyMemoryview_FromBuffer(info) is called, it should be OK to discard info by calling PyBuffer_Release() Now I'm puzzled: I thought your goal was to preserve the implicit cleanup from 3.2, i.e. PyBuffer_Release() is called when the managed buffer is deallocated. Without the patch it's OK to call PyBuffer_Release(info) after PyMemoryview_FromBuffer(info). With the patch you can't call PyBuffer_Release(info), since it's done automatically already. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15821 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15821] PyMemoryView_FromBuffer() behavior change (possible regression)
Alexander Belopolsky added the comment: On Fri, Aug 31, 2012 at 3:12 PM, Stefan Krah rep...@bugs.python.org wrote: Now I'm puzzled: I thought your goal was to preserve the implicit cleanup from 3.2, i.e. PyBuffer_Release() is called when the managed buffer is deallocated. The issue that I raised in msg169472 above was that PyMemoryView_FromBuffer() would not copy .obj from Py_buffer structure to the memoryview. A related issue is that it looks like PyObject_GetBuffer() often does not fill .obj either. I would expect that PyObject_GetBuffer() would always store a new reference in .obj to assure that the .buf pointer remains valid until PyBuffer_Release() is called explicitly. (I am ignoring the issue of mutable objects such as lists for the moment.) PyMemoryView_FromBuffer() in turn should store an additional reference in its own private copy of Py_buffer structure. After PyMemoryView_FromBuffer() returns a well-behaved program should call PyBuffer_Release() releasing the first reference and the second reference should be released in memoryview destructor. Am I missing something? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15821 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15821] PyMemoryView_FromBuffer() behavior change (possible regression)
Alexander Belopolsky added the comment:
Here is what I think the test case should look like (untested):
static PyObject *
memoryview_from_buffer_cleanup(PyObject *self, PyObject *noargs)
{
PyObject *b, *view = NULL;
Py_buffer info;
Py_ssize_t shape[3] = {2, 2, 3};
Py_ssize_t strides[3] = {6, 3, 1};
const char *cp = abcdefghijkl;
b = PyBytes_FromString(cp);
if (b == NULL)
return NULL;
if (PyObject_GetBuffer(b, info, PyBUF_FULL_RO) 0)
goto done;
/* reshape */
info.ndim = 3;
info.shape = shape;
info.strides = strides;
view = PyMemoryView_FromBuffer(info);
/* release resources allocated for Py_buffer struct
before it goes out of scope */
PyBuffer_Release(info);
done:
Py_DECREF(b);
return view;
}
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue15821
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
