Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buffered io seek() buggy #51889

Closed
pakal mannequin opened this issue Jan 5, 2010 · 11 comments
Closed

buffered io seek() buggy #51889

pakal mannequin opened this issue Jan 5, 2010 · 11 comments
Labels
topic-IO type-bug An unexpected behavior, bug, or error

Comments

@pakal
Copy link
Mannequin

pakal mannequin commented Jan 5, 2010

BPO 7640
Nosy @amauryfa, @pitrou, @pakal
Files
  • Py26_relative_seek.patch
  • 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 2010-05-15.20:33:38.135>
    created_at = <Date 2010-01-05.19:38:09.464>
    labels = ['type-bug', 'expert-IO']
    title = 'buffered io seek() buggy'
    updated_at = <Date 2010-05-15.20:33:38.133>
    user = 'https://github.com/pakal'

    bugs.python.org fields:

    activity = <Date 2010-05-15.20:33:38.133>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2010-05-15.20:33:38.135>
    closer = 'pitrou'
    components = ['IO']
    creation = <Date 2010-01-05.19:38:09.464>
    creator = 'pakal'
    dependencies = []
    files = ['16085']
    hgrepos = []
    issue_num = 7640
    keywords = ['patch']
    message_count = 11.0
    messages = ['97275', '97276', '97287', '97302', '97303', '97305', '97307', '98681', '98682', '105796', '105822']
    nosy_count = 3.0
    nosy_names = ['amaury.forgeotdarc', 'pitrou', 'pakal']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue7640'
    versions = ['Python 2.6']

    @pakal
    Copy link
    Mannequin Author

    pakal mannequin commented Jan 5, 2010

    I've noticed a severe bug in my python 2.6.2 io module, and looking at _buffered_raw_seek in http://svn.python.org/view/python/trunk/Modules/_io/bufferedio.c?view=markup, it seems the bug is still there in the C reimplementation of buffered io classes.

    The problem is, if we seek a buffered stream with whence=os.SEEK_CUR, that call is directly transmitted to the underlying raw stream, which has not the same file pointer position as the buffered stream. So in the end, the file pointer doesn't get moved at the right absolute offset, but at an offset which depends of the state of the read head buffer or the write buffer.
    When using os.SEEK_CUR, it would be necessary to call tell() or to manually compute offsets, so that the seek() works as expected.

    Regards,
    Pascal

    @pakal pakal mannequin added topic-IO type-bug An unexpected behavior, bug, or error labels Jan 5, 2010
    @pitrou
    Copy link
    Member

    pitrou commented Jan 5, 2010

    Could you construct a test case?
    (it's even better if you fix the offending code as well of course)

    @amauryfa
    Copy link
    Member

    amauryfa commented Jan 6, 2010

    The problem seems to be fixed in trunk (the future 2.7).
    BufferedRandom.seek() contains a block labelled "# Undo read ahead." which does not exists in 2.6.

    I reproduce the problem with a file opened with mode "rb+"

    from io import open
    open("tmpfile", "wb").write("abcdefgh" * 1024 + "X")
    f = open("tmpfile", "rb+")
    print f.read(1) # "a"
    f.seek(0, 1)
    print f.read(1) # should be "b", 2.6 returns "X"

    @pakal
    Copy link
    Mannequin Author

    pakal mannequin commented Jan 6, 2010

    My bad, I had looked at _buffered_raw_seek, not buffered_seek >_<
    Indeed, the code is OK in both trunk _io an _pyio modules.
    And the SEEK_CUR flag (value : 1) seems more than sufficiently tested in test_io actually, for example in the function write_ops() :
    http://svn.python.org/view/python/trunk/Lib/test/test_io.py?view=markup

    So concerning python2.6, isn't that just possible to backport _pyio (and its test suite) to it (renamed as io.py) ? They seem to offer the same functionality, except that _pyio is much more robust than io.py (the code around seek(), in particular, is much more complete).
    Just tell me, else I'll gather a patch for the seek() problem.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 6, 2010

    So concerning python2.6, isn't that just possible to backport _pyio
    (and its test suite) to it (renamed as io.py) ?

    I would not be against it, but someone has to provide a patch.
    The current _pyio.py might rely on other new behaviour of 2.7, so
    perhaps it won't be that easy.

    @pakal
    Copy link
    Mannequin Author

    pakal mannequin commented Jan 6, 2010

    Hum, with a selective merge (tortoiseSVN makes it easy), backporting _pyio should be doable in a decent time. Triaging pertinent tests should be more brain damaging :p
    I'm looking at this.

    @pakal
    Copy link
    Mannequin Author

    pakal mannequin commented Jan 6, 2010

    Well, here is a patch for the seek() methods of io module, in python2.6 maintenance branch.
    Finally, I've only backported some assertions and the offset stuffs - I'm not comfortable enough about recent io refactorings to do more (it changed pretty quickly while I looked away, actually :p).
    Tests have been patched too (bufferedrandom wasn't tested as other buffer classes), and to have the whole suite pass, I had to modify testWriteNonBlocking() as well (mock objects returned wrong values - this test is marked as unreliable in sources anyway).
    If more is needed to patch the py2.6 branch, just tell me B-)

    @pakal
    Copy link
    Mannequin Author

    pakal mannequin commented Feb 1, 2010

    BufferedRandom's seek() is still borken in the latest svn revision of python2.6 (I haven't checked python2.7 yet), so here is a new, smaller patch, tested with the 6 IO-related test suites this time, on win32 and linux.

    @pakal
    Copy link
    Mannequin Author

    pakal mannequin commented Feb 1, 2010

    The patch itself...

    @pakal
    Copy link
    Mannequin Author

    pakal mannequin commented May 15, 2010

    Hello

    I advocate the inclusion of this patch to the 2.6 maintenance branch, because currently the io module in this branch (which is still the most recent 2.X version released) is simply broken.
    People using it will certainly encounter MAJOR file corruptions due to the buggy seek().

    Or should we rather backport the much more evoluated io of trunk to py2.6 ?

    @pitrou
    Copy link
    Member

    pitrou commented May 15, 2010

    Committed in r81203. Thank you, Pascal!

    @pitrou pitrou closed this as completed May 15, 2010
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-IO type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants