[issue26720] memoryview from BufferedWriter becomes garbage

[Martin Panter]

Martin Panter added the comment:

Closing this as a duplicate of Issue 15994, where I have proposed a patch to 
add a note for RawIOBase.write(), and call memoryview.release() when the method 
returns.

--
resolution:  -> duplicate
status: open -> closed
superseder:  -> memoryview to freed memory can cause segfault

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26720] memoryview from BufferedWriter becomes garbage

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Python implementation passes bytearray to underlying write and delete it's 
content just after that. Thus saving written data was never worked, and making 
it working don't worth efforts. I agree with you, we should add a warning 
against saving. This might be a part of issue20699.

As for original issue, this is potential crash, and we should fix this.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26720] memoryview from BufferedWriter becomes garbage

[Martin Panter]

Martin Panter added the comment:

The trouble with my original idea is that it is complex to implement, and 
inconsistent. If you wrote small amounts to your BufferedWriter, you would get 
a memoryview of bytes that you save for later. If there was a write of a large 
bytes object, we could pass a memoryview of that bytes object (or the actual 
bytes object). But if there was a write of a large bytearray, we could end up 
locking that bytearray, preventing it from being resized, and there could be 
problems mutating the bytearray.

Because I can’t see how to eliminate all inconsistencies, I prefer to document 
against saving the memoryview for later.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26720] memoryview from BufferedWriter becomes garbage

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I meant getting rid of memoryview at all and passing bytes buffer object 
directly to underlying write. But now I see that this idea perhaps is not 
feasible since we must write not only from the start of the buffer.

A writer like AuditableBytesIO is very attractive implementation. I used it 
multiple times, especially in tests. Your initial proposition LGTM, and perhaps 
this is only the solution applicable in maintained releases. Thus we should 
implement it in any case. In 3.6 we can add a warning.

Note that there are two kinds of references to a buffer: a reference to 
memoryview object and new memoryview that refers to the same buffer.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26720] memoryview from BufferedWriter becomes garbage

[Martin Panter]

Martin Panter added the comment:

I realize there is another problem, and doing tricks with a bytes object won’t 
help that. BufferedWriter bypasses its own buffer for large writes:

>>> writer = BufferedWriter(Raw())
>>> large = bytearray(1)
>>> writer.write(large)
1
>>> written.tobytes()[:10]
b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
>>> large[:5] = b"blaua"
>>> written.tobytes()[:10]
b'blaua\x00\x00\x00\x00\x00'

BufferedWriter is passing a view of the original input through, without any 
copying. Perhaps the simplest thing is to warn and prevent the user from 
accessing the buffer after write() returns. I suggested some imperfect ideas in 
Issue 15994. Maybe I should just close this as a duplicate.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26720] memoryview from BufferedWriter becomes garbage

[Martin Panter]

Martin Panter added the comment:

To create a memoryview with unlimited lifetime, I understand we need to 
nominate an “exporting object”, which becomes memoryview.obj. Using a bytes 
object here might be the simplest fix for just BufferedWriter.

However it looks like the buffer is shared with BufferedReader and others. To 
fix the analogous bug with BufferedReader, a bytearray might be better, because 
the user could see it being mutated when reading into the memoryview.

I think bytearray might be okay for BufferedWriter too, as long as we prevent 
it being resized. The user would be able to alter the contents of the buffer, 
but I don’t see that as a problem. An alternative would be a new opaque object 
that doesn’t do much except have a reference count.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26720] memoryview from BufferedWriter becomes garbage

[Martin Panter]

Martin Panter added the comment:

On further thought, I think releasing the buffer would not be the best 
long-term solution. I encountered this bug when wrapping custom 
AuditableBytesIO objects (Lib/test/test_httpservers.py) with BufferedWriter, 
where the raw object just saves each write() buffer in a list for later use.

Serhiy: what you say sounds like what I had in mind, except I suspect it 
doesn’t matter whether the memoryview is backed by a bytes object or something 
else. The main point is we allocate a new buffer if the old one is still 
referenced by the memoryview.

It seems this problem has already been discovered, along with BufferedReader 
and PyUnicode_Decode(): . The 
BufferedReader case can have more serious consequences because it is writable 
(Issue 15994).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26720] memoryview from BufferedWriter becomes garbage

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Another option is to use the bytes object as internal buffer. If its refcount 
is == 1 (normal case), it is modified in-place, otherwise (if the reference is 
saved outside) new bytes object is created.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26720] memoryview from BufferedWriter becomes garbage

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
nosy: +benjamin.peterson, pitrou, serhiy.storchaka, stutzbach

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26720] memoryview from BufferedWriter becomes garbage

[Martin Panter]

New submission from Martin Panter:

>>> class Raw(RawIOBase):
... def writable(self): return True
... def write(self, b):
... global written
... written = b
... return len(b)
... 
>>> writer = BufferedWriter(Raw())
>>> writer.write(b"blaua")
5
>>> raw = writer.detach()
>>> written

>>> written.tobytes()
b'blaua'
>>> del writer
>>> written.tobytes()  # Garbage
b'\x80f\xab\x00\x00'

Assuming this is pointing into unallocated memory, maybe it could trigger a 
segfault, though I haven’t seen that.

I haven’t looked at the implementation. But I am guessing that BufferedWriter 
is passing a view of its internal buffer to write(). For Python 2, perhaps the 
fix is to check if that memoryview is still referenced, and allocate a new 
buffer if so. 3.5 should probably inherit this fix.

Another option for 3.6 might be to call release() when write() returns. This 
should be documented (along with the fact that memoryview is possible in the 
first place; see Issue 20699).

--
components: IO
messages: 263083
nosy: martin.panter
priority: normal
severity: normal
status: open
title: memoryview from BufferedWriter becomes garbage
type: behavior
versions: Python 2.7, Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com