Issue1793

Doing a file.seek(N, 1) fails for files opened in read/write mode ("r+" or "rb+"). This first manifested for me with mutagen corrupting m4a files when writing tags.


Output from the attached script (on two different systems, Linux and OS-X) gives:

--- OS-X ---
$ python ./script/test-seek.py 
Python 2.6.1
Darwin Kernel Version 10.8.0: Tue Jun  7 16:32:41 PDT 2011; root:xnu-1504.15.3~1/RELEASE_X86_64
No errors

$ jython ./script/test-seek.py 
Jython 2.5.2
Java HotSpot(TM) 64-Bit Server VM, 20.1-b02-384, Apple Inc.
Traceback (most recent call last):
  File "./script/test-seek.py", line 31, in <module>
    assert f.tell() == x, "before read: pos should be %d but was %d" % (x, f.tell())
AssertionError: before read: pos should be 253 but was 254


--- Linux ---
$ python script/test-seek.py 
Python 2.7.1+
#42-Ubuntu SMP Mon Apr 11 03:31:50 UTC 2011
No errors

$ java -jar target/lib/jython-standalone-2.5.2.jar script/test-seek.py 
Jython 2.5.2
OpenJDK Server VM, 20.0-b11, Sun Microsystems Inc.
Traceback (most recent call last):
  File "script/test-seek.py", line 31, in <module>
    assert f.tell() == x, "before read: pos should be %d but was %d" % (x, f.tell())
AssertionError: before read: pos should be 253 but was 254

Target beta 4

Attaching a unittest test case (that will fail) to reproduce this in regrtest.

Huh. Looks like this is caused by optimization in the way we buffer file read operations. fd.read(1) does not strictly read only one byte from the underlying java.nio.channels.FileChannel object.

Jython's fd.read() (where fd is returned by file() built-in function) is dispatched to PyFile#file_read method. Eventually, the read operation is done by calling FileChannel#read(ByteBuffer[] dsts) scattered IO read where the first byte buffer is the one returned to the caller (in this case of size 1), but there exists a secondary byte buffer that will be filled with the rest of the file.

So in the case of the test case above, when we're at, say, position 253 and call fd.read(1), we'll get the 254th byte as read, but the underlying file channel has been read all the way to the end. Hence, fd.seek(-1, SEEK_CUR) having the surprising result.

One way I can think of to address this is to introduce a method overloading BufferedIOBase#readinto() which can disable the buffering behavior described above. And then, have #read(int size) use the unbuffered method. I figured this might be appropriate since the caller specifies how many bytes to read explicitly, so we shouldn't assume to much and read beyond that.

Santoso, the approach in general looks good, but it's currently failing on a sequence of reads in test_builtin.BuiltinTest.test_open, likely due to some buffering behavior.

I see. Thanks for giving it a first look. I'll see what I can do.

On Fri, Mar 20, 2015, 11:31 PM Jim Baker <report@bugs.jython.org> wrote:

>
> Jim Baker added the comment:
>
> Santoso, the approach in general looks good, but it's currently failing on
> a sequence of reads in test_builtin.BuiltinTest.test_open, likely due to
> some buffering behavior.
>
> _______________________________________
> Jython tracker <report@bugs.jython.org>
> <http://bugs.jython.org/issue1793>
> _______________________________________
>

Attaching a patched patch.

Santoso, thanks, this now looks great and passes the regrtest! Please commit so it can be part of the release candidate, which Frank is just about to build.

I'm very glad we could have this last fix in!

Committed with changeset b597f478d5ab.

Although the changeset https://hg.python.org/jython/rev/b597f478d5ab was linked to #1011, it's fixed regardless. Marking accordingly.

I have no idea how I got #1011. I'm going to push a trivial change changing test_issue1011 method name to test_issue1793.