[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Changes by STINNER Victor victor.stin...@gmail.com: -- Removed message: http://bugs.python.org/msg172160 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Roundup Robot added the comment: New changeset b7f144d14798 by Victor Stinner in branch '3.4': Issue #16133: The asynchat.async_chat.handle_read() method now ignores http://hg.python.org/cpython/rev/b7f144d14798 New changeset aa150c7a5d24 by Victor Stinner in branch 'default': (Merge 3.4) Issue #16133: The asynchat.async_chat.handle_read() method now http://hg.python.org/cpython/rev/aa150c7a5d24 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Roundup Robot added the comment: New changeset d422062d7d36 by Victor Stinner in branch '2.7': Issue #16133: The asynchat.async_chat.handle_read() method now ignores http://hg.python.org/cpython/rev/d422062d7d36 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
STINNER Victor added the comment: I modified EWOULDBLOCK.patch to use BlockingIOError on Python 3 and I added a unit test. I also added EALREADY and EINPROGRESS which are used by the BlockingIOError in Python 3, just in case. Thanks Xavier for your patch, sorry for the delay. -- resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
STINNER Victor added the comment: EWOULDBLOCK.patch: asyncio ignores BlockingIOError on sock.recv(), except BlockingIOError: is more portable and future proof than _RETRY = frozenset((EWOULDBLOCK, EAGAIN)). Except of that, EWOULDBLOCK.patch change looks correct. -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
STINNER Victor added the comment: Modifying recv() to return None doesn't look correct. I read it as: you should always use recv() output, except if the result is None: in this case, do nothing. In Python, we use exceptions for that. BUT in fact, sock.recv() already raises an exception, so asyncore should not convert the exception to a magic value (None). Modifying the behaviour of recv() in asyncore breaks the backward compatibility. Returning None makes it harder to write asyncore code working on Python 3.4 and 3.5 (if the change is done in Python 3.5). I prefer EWOULDBLOCK.patch approach: document the issue in asyncore documentation and handle BlockingIOError in asynchat. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
STINNER Victor added the comment: See also the issue #15982 which is the exactly the same on Windows. (By the way, the asyncore module has been marked as deprecated in Python 3.4 in favor of asyncio, and this issue is already solved in asyncio.) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Mark Lawrence added the comment: Could we have a review of the latest path from Xavier please. Aside, msg172160 add Windows project file for _sha3 module. I choose to build _sha3 as a sparat module as it's rather large (190k for AMD64)., presumably Christian mistyped an issue number? -- nosy: +BreamoreBoy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Xavier de Gaye added the comment: For the reson why read() must still check for EWOULDBLOCK even though after select() has reported a file descriptor ready for reading, see the BUGS section of select linux man page, which says: Under Linux, select() may report a socket file descriptor as ready for reading, while nevertheless a subsequent read blocks. This could for example happen when data has arrived but upon examination has wrong checksum and is discarded. There may be other circumstances in which a file descriptor is spuriously reported as ready. Thus it may be safer to use O_NONBLOCK on sockets that should not block. -- nosy: +xdegaye ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Xavier de Gaye added the comment: Deciding what's best to do at this point without breaking existent code is not easy, that is why I think that on python = 3.3 we should fix *asynchat* in order to take EAGAIN/EWOULDBLOCK into account and leave asyncore's recv() alone. IMHO for all python versions, asynchat should be fixed and recv() left unchanged raising OSError with EAGAIN/EWOULDBLOCK. With the proposed change on recv(), asyncore users would need to handle this new None returned value anyway, instead of handling the exception which is much more explicit. The attached patch does this on the default branch. -- Added file: http://bugs.python.org/file30193/EWOULDBLOCK.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Nidan added the comment: Why should asynchat.handle_read care about closed sockets if asyncore.recv does that already? Currently asynchat.handle_read handles empty strings from asycore.recv gracefully (by doing some unnecessary work aka executing the remainder of the function), it doesn't treat them specially. The only path that might cause asynchat.handle_read to close the socket requires asycore.recv to throw. Introducing None as possible return value from asyncore.recv therefore seems unnecessary to me. Changed the patch accordingly. -- Added file: http://bugs.python.org/file30088/issue16133.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Giampaolo Rodola' added the comment: recv() returning an empty string has always been an alias for connection lost though, that is why it cannot be used and I was proposing returning a new type in Python 3.4. Point is we're paying a bad design decision: asyncore shouldn't have asked the user to call recv() directly in the first place and call a data_received(chunk) callback method instead. Deciding what's best to do at this point without breaking existent code is not easy, that is why I think that on python = 3.3 we should fix *asynchat* in order to take EAGAIN/EWOULDBLOCK into account and leave asyncore's recv() alone. The issue would still exist but it would be mitigated by the fact that who wants to write a protocol is likely to use asynchat, not asyncore. As for Python 3.4 we can: - make asyncore's recv() return None and document it - deprecate recv() - introduce data_received(chunk) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Giampaolo Rodola' added the comment: I confirm I can reproduce this issue also in pyftpdlib: https://code.google.com/p/pyftpdlib/issues/detail?id=255 Current proposed patch returning '' is not ideal as for asynchat '' is an alias for 'connection lost'. In summary recv() in case of EAGAIN should return None and asynchat.handle_read() should take that into account. Will provide a patch later. -- assignee: - giampaolo.rodola ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Giampaolo Rodola' added the comment: Patch is in attachment. I'm a bit worried about Python versions 3.4 because this kinds of breaks backward compatibility as recv() is not expected to return None but I see no other saner solution. -- keywords: +patch Added file: http://bugs.python.org/file30026/issue16133.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Roundup Robot added the comment: New changeset 6a1d8e78b23d by Christian Heimes in branch 'default': Issue #16133: add Windows project file for _sha3 module. I choose to build _sha3 as a sparat module as it's rather large (190k for AMD64). http://hg.python.org/cpython/rev/6a1d8e78b23d -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
New submission from Nidan: I recently had lots of the following exception: error: uncaptured python exception, closing channel servercore_persistent.ConnectionHandler connected 127.0.0.1:53609 at 0x8d27eec (class 'socket.error':[Errno 11] Resource temporarily unavailable [/usr/lib/python2.7/asynchat.py|handle_read|110] [/usr/lib/python2.7/asyncore.py|recv|384]) Error 11 is EAGAIN or EWOULDBLOCK, so asyncore/asynchat tries to read from a nonblocking socket which has no data available. Since this is a temporary error the socket shouldn't be closed. The bug can be fixed by changing asyncore.dispatcher.recv to def recv(self, buffer_size): try: data = self.socket.recv(buffer_size) if not data: # a closed connection is indicated by signaling # a read condition, and having recv() return 0. self.handle_close() return '' else: return data except socket.error, why: # winsock sometimes throws ENOTCONN if why.args[0] in _DISCONNECTED: self.handle_close() return '' elif why.args[0] in (EAGAIN, EWOULDBLOCK): return '' else: raise While looking at the source I also saw that asyncore.dispatcher.send and .connect check against EWOULDBLOCK but not against EAGAIN. Since both constants may have different values and POSIX allows to return either of them these functions should check against both error constants. -- components: Library (Lib) messages: 171967 nosy: Nidan priority: normal severity: normal status: open title: asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK type: behavior versions: Python 2.7, Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue16133] asyncore.dispatcher.recv doesn't handle EAGAIN / EWOULDBLOCK
Changes by Antoine Pitrou pit...@free.fr: -- nosy: +giampaolo.rodola, josiahcarlson, stutzbach versions: +Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
