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

Raw I/O writelines() broken for non-blocking I/O #70480

Closed
vstinner opened this issue Feb 5, 2016 · 8 comments
Closed

Raw I/O writelines() broken for non-blocking I/O #70480

vstinner opened this issue Feb 5, 2016 · 8 comments
Labels

Comments

@vstinner
Copy link
Member

vstinner commented Feb 5, 2016

BPO 26292
Nosy @pitrou, @vstinner, @vadmium, @jstasiak
Superseder
  • bpo-13322: The io module doesn't support non-blocking files
  • Files
  • writelines-doc.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 2019-10-10.08:27:39.173>
    created_at = <Date 2016-02-05.08:44:20.262>
    labels = ['expert-IO']
    title = 'Raw I/O writelines() broken for non-blocking I/O'
    updated_at = <Date 2019-10-10.08:27:39.172>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2019-10-10.08:27:39.172>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-10-10.08:27:39.173>
    closer = 'vstinner'
    components = ['IO']
    creation = <Date 2016-02-05.08:44:20.262>
    creator = 'vstinner'
    dependencies = []
    files = ['43488']
    hgrepos = []
    issue_num = 26292
    keywords = ['patch']
    message_count = 8.0
    messages = ['259640', '259641', '259655', '259656', '259690', '268413', '268900', '354338']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'vstinner', 'martin.panter', 'raylu', 'jstasiak']
    pr_nums = []
    priority = 'normal'
    resolution = 'duplicate'
    stage = 'resolved'
    status = 'closed'
    superseder = '13322'
    type = None
    url = 'https://bugs.python.org/issue26292'
    versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

    @vstinner
    Copy link
    Member Author

    vstinner commented Feb 5, 2016

    Copy of Antoine Pitrou's email (sent in 2012 ;-):
    https://mail.python.org/pipermail/python-dev/2012-August/121396.html

    Hello,

    I was considering a FileIO.writelines() implementation based on
    writev() and I noticed that the current RawIO.writelines()
    implementation is broken: RawIO.write() can return a partial write but
    writelines() ignores the result and happily proceeds to the next
    iterator item (and None is returned at the end).

    (it's probably broken with non-blocking streams too, for the same
    reason)

    In the spirit of RawIO.write(), I think RawIO.writelines() could return
    the number of bytes written (allowing for partial writes).

    Regards

    Antoine.

    --
    Software development and contracting: http://pro.pitrou.net

    @vstinner
    Copy link
    Member Author

    vstinner commented Feb 5, 2016

    "(it's probably broken with non-blocking streams too, for the same
    reason)"

    Yes it is :-)

    We hitted this issue on eventlet when I changed the socket.send() method in eventlet 0.18 to stop sending data on partial write, whereas eventlet < 0.18 used a loop to ensure that all bytes are written.

    The side effect of my change is that (indirectly) socket.makefile().writelines() may also use partial write if the underlying send() only writes partially data.

    The problem is that writelines() has *no* result, so the caller cannot be aware of the partial write and data is lost...

    eventlet:

    "In the spirit of RawIO.write(), I think RawIO.writelines() could return
    the number of bytes written (allowing for partial writes)."

    I don't know yet what is the best option for eventlet, but we should probaly enhance the Python stdlib io module to return something on writelines() and *document* the behaviour on partial write.

    The problem writelines() API is that writelines() not only takes a single text/bytes string, but a *list* of text/bytes strings.

    Another option is to modify to retry on partial write to ensure that all data is written. If the underlying write() method returns None (blocking I/O error), we must raise an error. It doesn't make sense to retry writing on a non-blocking I/O error.

    In this case, we must document that writelines() *must not* be used on non-blocking I/O (ex: non-blocking socket, pipe, whatever).

    @vadmium
    Copy link
    Member

    vadmium commented Feb 5, 2016

    Is deprecating RawIOBase.writelines() an option, and only recommending BufferedIOBase.writelines() and TextIOBase.writelines()?

    Otherwise, I think it would make most sense to keep retrying until all the data is written. This mirrors how I understand readline() and readall() work (keeps reading until it gets as much as necessary).

    For non-blocking mode, readline() does not support that (see bpo-13858). It does not make much sense to me to have writelines() support non-blocking mode either.

    @vstinner
    Copy link
    Member Author

    vstinner commented Feb 5, 2016

    Is deprecating RawIOBase.writelines() an option, and only recommending BufferedIOBase.writelines() and TextIOBase.writelines()?

    IMHO the problem is the same for other classes.

    @vadmium
    Copy link
    Member

    vadmium commented Feb 5, 2016

    For BufferedIOBase, the documentation says write() will not return a short number of bytes, so I don’t see how writelines() is a problem there. For TextIOBase, the documentation is not so clear <https://docs.python.org/release/3.4.3/library/io.html#io.TextIOBase.write\>, but I understand the standard library implementations do not do short writes.

    @vstinner vstinner changed the title Raw I/O writelines() broken Raw I/O writelines() broken for non-blocking I/O Feb 11, 2016
    @vadmium
    Copy link
    Member

    vadmium commented Jun 13, 2016

    Victor, why did you change the title to specify non-blocking mode? I think blocking mode can also be handled at the same time.

    I propose:

    1. In existing versions (2.7, 3.5): Document that it is undefined what IOBase.writelines() will do if a write() call does a partial write, returns None, or encounters a blocking error. Explicitly document that BufferedIOBase.writelines() and TextIOBase.writelines() will completely write all the data passed, or raise an exception. Document that BlockingIOError.characters_written is undefined even for BufferedIOBase.writelines().

    2. Commit my bpo-26721 change to socketserver, so that StreamRequestHandler.wfile implements BufferedIOBase instead of RawIOBase.

    3. In a new version (3.6): Deprecate IOBase.writelines() and thus RawIOBase.writelines(), in favour of either using BufferedIOBase, or manually calling write(). Emit a DeprecationWarning, but add BufferedIOBase and TextIOBase implementations that do not emit the warning.

    BufferedIOBase.writelines() could be fixed to report the correct BlockingIOError.characters_written value, but that could be handled as a separate bug if anyone cares.

    @vadmium
    Copy link
    Member

    vadmium commented Jun 20, 2016

    Here is a patch documenting that RawIOBase.writelines() is undefined for partial writes and blocking errors, as mentioned in (1) above.

    The alternative of requiring write() to be retried would IMO be unfair to existing writelines() implementations. It also seems out of place considering we don’t even have a RawIOBase.writeexactly() type of method.

    @vstinner
    Copy link
    Member Author

    I close this issue as duplicate of bpo-13322 which is older and has a longer history.

    @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
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants