Repository navigation
PEP-3118: remove obsolete write-locks #58411
Description
| BPO | 14203 |
|---|---|
| Nosy | @ncoghlan, @pitrou, @skrah, @serhiy-storchaka |
| Files |
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 = <Date 2015-02-04.13:51:46.544>
created_at = <Date 2012-03-05.18:32:48.582>
labels = ['interpreter-core', 'performance']
title = 'PEP-3118: remove obsolete write-locks'
updated_at = <Date 2015-02-04.13:51:46.543>
user = 'https://github.com/skrah'bugs.python.org fields:
activity = <Date 2015-02-04.13:51:46.543>
actor = 'skrah'
assignee = 'none'
closed = True
closed_date = <Date 2015-02-04.13:51:46.544>
closer = 'skrah'
components = ['Interpreter Core']
creation = <Date 2012-03-05.18:32:48.582>
creator = 'skrah'
dependencies = []
files = ['24738', '38003', '38004']
hgrepos = []
issue_num = 14203
keywords = ['patch', 'needs review']
message_count = 15.0
messages = ['154969', '154971', '154972', '219995', '235346', '235348', '235352', '235354', '235355', '235358', '235359', '235360', '235361', '235363', '235386']
nosy_count = 6.0
nosy_names = ['ncoghlan', 'pitrou', 'skrah', 'python-dev', 'serhiy.storchaka', 'kelleynnn']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue14203'
versions = ['Python 3.5']
Activity
skrah commentedon Mar 5, 2012
bytearray_getbuffer() checks for view==NULL. But this has never been
allowed:
PEP: "The second argument is the address to a bufferinfo structure.
Both arguments must never be NULL."
DOCS (3.2): "view must point to an existing Py_buffer structure
allocated by the caller".
A quick grep through the source tree shows no instances where
the middle argument of either PyObject_GetBuffer of bf_getbuffer
is NULL or 0.
Patch attached, all tests pass. I wouldn't be comfortable to
commit it without review though (it's just too strange).
BTW, the next conditional in bytearray_getbuffer ...
is also superfluous, since PyBuffer_FillInfo() cannot fail
if readonly==0.
skrah commentedon Mar 5, 2012
array_buffer_getbuf does a similar thing:
if (view==NULL) goto finish;
finish:
self->ob_exports++; // ???
return 0
Where does this view==NULL thing come from?
skrah commentedon Mar 5, 2012
I seems to be a feature to get a "lock" on an exporter
without the exporter filling in a buffer. It was there
from the beginning:
http://svn.python.org/view/python/branches/release30-maint/Objects/bytearrayobject.c?r1=56849&r2=57181
Last use that I can see is here:
http://hg.python.org/cpython/file/df3b2b5db900/Modules/posixmodule.c#l561
kelleynnn commentedon Jun 7, 2014
I have verified that this feature is unused in the source tree; in fact, there are no internal calls to bytearray_getbuffer() at all. The only thing bytearray_getbuffer() does with its second arg is pass it to PyBuffer_FillInfo(), which immediately checks it and passes 0 up the call stack if it is NULL. (A comment in PyBuffer_FillInfo() asks why -1 is not passed up instead; it's probably to distinguish this feature from the error condition handled in the immediately following conditional block.)
There are potentially other issues stemming from this legacy feature in bytearray_getbuffer(), PyBuffer_FillInfo(), and elsewhere. The maintainers may see fit to open tickets on these issues as well.
There's more relevant commentary on the feature here: http://comments.gmane.org/gmane.comp.python.devel/130521
skrah commentedon Feb 3, 2015
New patch with tests.
python-dev commentedon Feb 3, 2015
New changeset e8fe32d43c96 by Stefan Krah in branch 'default':
Issue bpo-14203: Remove obsolete support for view==NULL in PyBuffer_FillInfo()
https://hg.python.org/cpython/rev/e8fe32d43c96
skrah commentedon Feb 3, 2015
bytesiobuf_getbuffer() also still has this obsolete feature, so
BufferError should be raised if view==NULL.
I'm unsure if this plays well with the new SHARED_BUF(b) thing.
Should the exception be raised before or after?
[-]bytearray_getbuffer: unnecessary code[/-][+]PEP-3118: remove obsolete write-locks[/+]serhiy-storchaka commentedon Feb 3, 2015
I think before. It doesn't harm, but it doesn't make much sense to unshare the buffer if its address is not used.
serhiy-storchaka commentedon Feb 3, 2015
E.g. the buffer should be unshared before incrementing b->exports, but if an exception is raised instead, there is no need to unshare the buffer before raising.
skrah commentedon Feb 3, 2015
Ok, thanks! bpo-14203-3.diff should be correct, then.
python-dev commentedon Feb 3, 2015
New changeset 0d5095a2422f by Stefan Krah in branch 'default':
Issue bpo-14203: Remove obsolete support for view==NULL in bytesiobuf_getbuffer()
https://hg.python.org/cpython/rev/0d5095a2422f
serhiy-storchaka commentedon Feb 3, 2015
Compiling failed on Windows: http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/5650
skrah commentedon Feb 3, 2015
Argh, the extern _PyBytesIOBuffer_Type hack. I knew it would cause some
trouble.
Any ideas how to test bytesiobuf_getbuffer() with a NULL view in
a more dignified way?
python-dev commentedon Feb 3, 2015
New changeset 17a8c5f8ca48 by Stefan Krah in branch 'default':
Issue bpo-14203: Temporary fix for the compile failure on Windows.
https://hg.python.org/cpython/rev/17a8c5f8ca48
skrah commentedon Feb 4, 2015
I think it's sufficient to test bytesiobuf_getbuffer() on
Linux and FreeBSD. The test just checks that the exception
is raised.