
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2014-11-18 14:02 byserhiy.storchaka, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| buffers.diff | serhiy.storchaka,2014-11-25 07:50 | review | ||
| buffers_2.patch | serhiy.storchaka,2014-12-19 17:02 | review | ||
| buffers_3.patch | serhiy.storchaka,2014-12-20 10:50 | review | ||
| Messages (18) | |||
|---|---|---|---|
| msg231323 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2014-11-18 14:02 | |
PyObject_AsCharBuffer(), PyObject_AsReadBuffer() and PyObject_AsWriteBuffer() release the buffer right after acquiring and return a pointer to released buffer. This is not safe and could cause issues sooner or later. These functions shouldn't be used in the stdlib at all. | |||
| msg231642 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2014-11-25 07:50 | |
Here is a patch which replaces deprecated functions with PyObject_GetBuffer() and like. It also introduce _PyBuffer_Converter for using in PyArgs_Parse* and clinic converter simple_buffer_converter which unlike to Py_buffer_converter ("y*") does not not force C contiguousity (is it correct?). | |||
| msg232941 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2014-12-19 17:02 | |
Updated patch addresses Antoine's comments.I still hesitate about C-contiguousity. Looks as all buffers created in the stdlib are C-contiguous, so we can't test non-contiguous buffers. Shouldn't PyObject_AsCharBuffer (or even PyObject_AsReadBuffer and_PyBuffer_Converter) accept only C-contiguous buffers? Who are buffer protocol experts? | |||
| msg232944 -(view) | Author: Antoine Pitrou (pitrou)*![]() | Date: 2014-12-19 17:54 | |
> Shouldn't PyObject_AsCharBuffer (or even PyObject_AsReadBuffer and_PyBuffer_Converter) accept only C-contiguous buffers?PyBUF_SIMPLE enforces contiguity. Seehttps://www.python.org/dev/peps/pep-3118/#access-flags andhttps://docs.python.org/3/c-api/buffer.html#c.Py_buffer.lenAlso Stefan's post athttp://mail.scipy.org/pipermail/numpy-discussion/2011-August/058189.htmlPerhaps Stefan can confirm. | |||
| msg232946 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2014-12-19 20:36 | |
Ah, there is a way to create non-contiguous buffers.>>> b = bytes(range(16))>>> m = memoryview(b)>>> m[::2].c_contiguousFalse> PyBUF_SIMPLE enforces contiguity.Then contiguousity check in getbuffer() inPython/getargs.c is redundant. And in most cases the use of _PyBuffer_Converter() and PyObject_GetBuffer() could be replaced by the use of "y*" and "w*" format units. | |||
| msg232947 -(view) | Author: Stefan Krah (skrah)*![]() | Date: 2014-12-19 20:43 | |
Yes, a PyBUF_SIMPLE request implies c-contiguous, so it's ok. | |||
| msg232968 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2014-12-20 10:48 | |
Here is simplified patch. _PyBuffer_Converter() and "simple_buffer" converter are gone. Fixed few leaks found by Martin. Fixed potential crash in ctypes. Fixed minor bugs and cleaned up ctypes tests for from_buffer(). | |||
| msg235197 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2015-02-01 18:55 | |
Could you please look at the patch Stefan? | |||
| msg235205 -(view) | Author: Stefan Krah (skrah)*![]() | Date: 2015-02-01 20:32 | |
[Slow internet connection, can't use Rietveld.]CDataType_from_buffer():I'm not that familiar with ctypes. What is the high level goal here?Allocate a chunk of memory, wrap it in a memoryview and have thememoryview release that memory when its refcount is 0?If so, I think we desperately need direct support for that inmemoryview. | |||
| msg235210 -(view) | Author: Martin Panter (martin.panter)*![]() | Date: 2015-02-01 21:52 | |
_CData.from_buffer() is meant to take a writable buffer, and create a “ctypes” object that shares the same memory. So it should not release the buffer until that “ctypes” object is no longer needed.However I don’t know the insides of memoryview() objects that well so I can’t say if the hack-the-memoryview code is correct or not. | |||
| msg235211 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2015-02-01 22:13 | |
from_buffer() uses a memory buffer of other object. It keeps a reference to the object to prevent deallocation of memory when there will be no more external references. But this doesn't prevent from reallocating of memory of living object (e.g. for resizing of bytearray). So we need to grab the buffer (with PyObject_GetBuffer) in from_buffer() and free it (with PyBuffer_Release) when it is no longer needed. menoryview can do this but needs a hack, because a memoryview created by PyMemoryView_FromBuffer() doesn't release the buffer. May be there is more official way? | |||
| msg235212 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2015-02-01 22:15 | |
Oh, Martin expressed the same thing better. | |||
| msg235256 -(view) | Author: Stefan Krah (skrah)*![]() | Date: 2015-02-02 13:09 | |
Thanks. No, I don't think there's an official way to accomplish that,but let's create one. How about a new function that takes the bufferrequest flags: PyMemoryView_FromObjectEx(exporter, PyBUF_SIMPLE|PyBUF_WRITABLE)If we can spare a new format code, this could be called directly inPyArg_ParseTuple(), which would give back the memoryview.Otherwise, you get the exporter from PyArg_ParseTuple() and callPyMemoryView_FromObjectEx() manually.If I'm not mistaken, this would save us the intermediate buffer on thestack, and it's more readable. | |||
| msg235257 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2015-02-02 13:15 | |
In any case we need a hack in 3.4. Let open new issue for adding PyMemoryView_FromObjectEx() or like. | |||
| msg235274 -(view) | Author: Stefan Krah (skrah)*![]() | Date: 2015-02-02 17:14 | |
Nice patch. I've found one issue (see Rietveld). I'm not sureabout 3.4 (the patch contains minor refactorings), but otherwiseI'd say go ahead with it. | |||
| msg235305 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2015-02-03 00:05 | |
New changeset1da9630e9b7f by Serhiy Storchaka in branch '3.4':Issue#22896: Avoid to use PyObject_AsCharBuffer(), PyObject_AsReadBuffer()https://hg.python.org/cpython/rev/1da9630e9b7fNew changeset2e684ce772de by Serhiy Storchaka in branch 'default':Issue#22896: Avoid to use PyObject_AsCharBuffer(), PyObject_AsReadBuffer()https://hg.python.org/cpython/rev/2e684ce772deNew changeset0024537a4687 by Serhiy Storchaka in branch 'default':Issue#22896: Fixed using _getbuffer() in recently added _PyBytes_Format().https://hg.python.org/cpython/rev/0024537a4687 | |||
| msg235318 -(view) | Author: Serhiy Storchaka (serhiy.storchaka)*![]() | Date: 2015-02-03 07:39 | |
Thanks Antoine and Stefan for your reviews. | |||
| msg253647 -(view) | Author: Martin Panter (martin.panter)*![]() | Date: 2015-10-29 00:32 | |
Please seeIssue 25498 for a crash possibly caused by the memoryview hack in CDataType_from_buffer(). | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:10 | admin | set | github: 67085 |
| 2015-10-29 00:32:09 | martin.panter | set | messages: +msg253647 |
| 2015-02-03 07:39:05 | serhiy.storchaka | set | status: open -> closed resolution: fixed messages: +msg235318 stage: patch review -> resolved |
| 2015-02-03 00:05:21 | python-dev | set | nosy: +python-dev messages: +msg235305 |
| 2015-02-02 17:14:31 | skrah | set | messages: +msg235274 |
| 2015-02-02 13:15:53 | serhiy.storchaka | set | messages: +msg235257 |
| 2015-02-02 13:09:51 | skrah | set | messages: +msg235256 |
| 2015-02-01 22:15:29 | serhiy.storchaka | set | messages: +msg235212 |
| 2015-02-01 22:13:01 | serhiy.storchaka | set | messages: +msg235211 |
| 2015-02-01 21:52:44 | martin.panter | set | messages: +msg235210 |
| 2015-02-01 20:32:24 | skrah | set | messages: +msg235205 |
| 2015-02-01 18:55:55 | serhiy.storchaka | set | messages: +msg235197 |
| 2014-12-20 10:50:57 | serhiy.storchaka | set | files: +buffers_3.patch |
| 2014-12-20 10:48:56 | serhiy.storchaka | set | messages: +msg232968 |
| 2014-12-19 23:59:11 | martin.panter | set | nosy: +martin.panter |
| 2014-12-19 20:43:18 | skrah | set | messages: +msg232947 |
| 2014-12-19 20:36:29 | serhiy.storchaka | set | messages: +msg232946 |
| 2014-12-19 17:54:49 | pitrou | set | nosy: +skrah messages: +msg232944 |
| 2014-12-19 17:03:03 | serhiy.storchaka | set | files: +buffers_2.patch messages: +msg232941 |
| 2014-12-18 15:40:34 | serhiy.storchaka | set | keywords: +needs review |
| 2014-12-06 23:52:48 | Arfrever | set | nosy: +Arfrever |
| 2014-11-28 16:18:04 | belopolsky | set | nosy: +belopolsky |
| 2014-11-25 07:50:36 | serhiy.storchaka | set | files: +buffers.diff nosy: +pitrou,larry messages: +msg231642 keywords: +patch stage: needs patch -> patch review |
| 2014-11-18 14:02:55 | serhiy.storchaka | create | |