homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Don't use PyObject_As*Buffer() functions
Type: resource usage Stage: resolved
Components: Extension Modules, Interpreter Core Versions: Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Arfrever, belopolsky, larry, martin.panter, pitrou, python-dev, serhiy.storchaka, skrah
Priority: normal Keywords: needs review, patch

Created on 2014年11月18日 14:02 by serhiy.storchaka, last changed 2022年04月11日 14:58 by admin. This issue is now closed.

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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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. See https://www.python.org/dev/peps/pep-3118/#access-flags and https://docs.python.org/3/c-api/buffer.html#c.Py_buffer.len
Also Stefan's post at http://mail.scipy.org/pipermail/numpy-discussion/2011-August/058189.html
Perhaps Stefan can confirm.
msg232946 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) 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_contiguous
False
> PyBUF_SIMPLE enforces contiguity.
Then contiguousity check in getbuffer() in Python/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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2015年02月01日 18:55
Could you please look at the patch Stefan?
msg235205 - (view) Author: Stefan Krah (skrah) * (Python committer) 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 the
memoryview release that memory when its refcount is 0?
If so, I think we desperately need direct support for that in
memoryview.
msg235210 - (view) Author: Martin Panter (martin.panter) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2015年02月01日 22:15
Oh, Martin expressed the same thing better.
msg235256 - (view) Author: Stefan Krah (skrah) * (Python committer) 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 buffer
request flags:
 PyMemoryView_FromObjectEx(exporter, PyBUF_SIMPLE|PyBUF_WRITABLE)
If we can spare a new format code, this could be called directly in
PyArg_ParseTuple(), which would give back the memoryview.
Otherwise, you get the exporter from PyArg_ParseTuple() and call
PyMemoryView_FromObjectEx() manually.
If I'm not mistaken, this would save us the intermediate buffer on the
stack, and it's more readable.
msg235257 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) 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) * (Python committer) Date: 2015年02月02日 17:14
Nice patch. I've found one issue (see Rietveld). I'm not sure
about 3.4 (the patch contains minor refactorings), but otherwise
I'd say go ahead with it.
msg235305 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015年02月03日 00:05
New changeset 1da9630e9b7f by Serhiy Storchaka in branch '3.4':
Issue #22896: Avoid to use PyObject_AsCharBuffer(), PyObject_AsReadBuffer()
https://hg.python.org/cpython/rev/1da9630e9b7f
New changeset 2e684ce772de by Serhiy Storchaka in branch 'default':
Issue #22896: Avoid to use PyObject_AsCharBuffer(), PyObject_AsReadBuffer()
https://hg.python.org/cpython/rev/2e684ce772de
New changeset 0024537a4687 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) * (Python committer) Date: 2015年02月03日 07:39
Thanks Antoine and Stefan for your reviews.
msg253647 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015年10月29日 00:32
Please see Issue 25498 for a crash possibly caused by the memoryview hack in CDataType_from_buffer().
History
Date User Action Args
2022年04月11日 14:58:10adminsetgithub: 67085
2015年10月29日 00:32:09martin.pantersetmessages: + msg253647
2015年02月03日 07:39:05serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg235318

stage: patch review -> resolved
2015年02月03日 00:05:21python-devsetnosy: + python-dev
messages: + msg235305
2015年02月02日 17:14:31skrahsetmessages: + msg235274
2015年02月02日 13:15:53serhiy.storchakasetmessages: + msg235257
2015年02月02日 13:09:51skrahsetmessages: + msg235256
2015年02月01日 22:15:29serhiy.storchakasetmessages: + msg235212
2015年02月01日 22:13:01serhiy.storchakasetmessages: + msg235211
2015年02月01日 21:52:44martin.pantersetmessages: + msg235210
2015年02月01日 20:32:24skrahsetmessages: + msg235205
2015年02月01日 18:55:55serhiy.storchakasetmessages: + msg235197
2014年12月20日 10:50:57serhiy.storchakasetfiles: + buffers_3.patch
2014年12月20日 10:48:56serhiy.storchakasetmessages: + msg232968
2014年12月19日 23:59:11martin.pantersetnosy: + martin.panter
2014年12月19日 20:43:18skrahsetmessages: + msg232947
2014年12月19日 20:36:29serhiy.storchakasetmessages: + msg232946
2014年12月19日 17:54:49pitrousetnosy: + skrah
messages: + msg232944
2014年12月19日 17:03:03serhiy.storchakasetfiles: + buffers_2.patch

messages: + msg232941
2014年12月18日 15:40:34serhiy.storchakasetkeywords: + needs review
2014年12月06日 23:52:48Arfreversetnosy: + Arfrever
2014年11月28日 16:18:04belopolskysetnosy: + belopolsky
2014年11月25日 07:50:36serhiy.storchakasetfiles: + buffers.diff

nosy: + pitrou, larry
messages: + msg231642

keywords: + patch
stage: needs patch -> patch review
2014年11月18日 14:02:55serhiy.storchakacreate

AltStyle によって変換されたページ (->オリジナル) /