[issue26685] Raise errors from socket.close()

STINNER Victor added the comment: New changeset 580cd5cd3603317d294a881628880a92ea221334 by Victor Stinner in branch '3.6': bpo-30319: socket.close() now ignores ECONNRESET (#2565) (#2566) https://github.com/python/cpython/commit/580cd5cd3603317d294a881628880a92ea221334 --

[issue26685] Raise errors from socket.close()

Changes by STINNER Victor : -- pull_requests: +2636 ___ Python tracker ___ ___

[issue26685] Raise errors from socket.close()

STINNER Victor added the comment: New changeset 67e1478dba6efe60b8e1890192014b8b06dd6bd9 by Victor Stinner in branch 'master': bpo-30319: socket.close() now ignores ECONNRESET (#2565) https://github.com/python/cpython/commit/67e1478dba6efe60b8e1890192014b8b06dd6bd9 --

[issue26685] Raise errors from socket.close()

Yury Selivanov added the comment: >> Would you mind to open an issue specific for asyncio? > I'll open an issue on GH today to discuss with you/Guido/asyncio devs. Guido, Victor, please take a look: https://github.com/python/asyncio/pull/449 -- ___

[issue26685] Raise errors from socket.close()

Yury Selivanov added the comment: > Ah, you became reasonable :-D Come on... :) > Would you mind to open an issue specific for asyncio? I'll open an issue on GH today to discuss with you/Guido/asyncio devs. -- ___ Python tracker

[issue26685] Raise errors from socket.close()

STINNER Victor added the comment: "After thinking about this problem for a while, I arrived to the conclusion that we need to fix asyncio." Ah, you became reasonable :-D "Essentially, when a user passes a socket object to the event loop API like 'create_server', they give up the control of

[issue26685] Raise errors from socket.close()

Yury Selivanov added the comment: After thinking about this problem for a while, I arrived to the conclusion that we need to fix asyncio. Essentially, when a user passes a socket object to the event loop API like 'create_server', they give up the control of the socket to the loop. The loop

[issue26685] Raise errors from socket.close()

Yury Selivanov added the comment: Maybe we should fix asyncio. Maybe if a user passes an existing socket object into loop.create_connection, it should duplicate the socket. -- ___ Python tracker

[issue26685] Raise errors from socket.close()

STINNER Victor added the comment: > My proposal: ignore EBADF in socket.close(). It means that the socket is > closed already. It doesn't matter why. As Martin explained, getting EBAD means that your code smells. Your code may close completely unrelated file descriptor... Say hello to race

[issue26685] Raise errors from socket.close()

Yury Selivanov added the comment: My proposal: ignore EBADF in socket.close(). It means that the socket is closed already. It doesn't matter why. -- ___ Python tracker

[issue26685] Raise errors from socket.close()

Yury Selivanov added the comment: Another example: some asyncio (run with uvloop) code: conn, _ = lsock.accept() f = loop.create_task( loop.connect_accepted_socket( proto_factory, conn)) # More code loop.run_forever()

[issue26685] Raise errors from socket.close()

Martin Panter added the comment: If libuv closes the FD (step 3), won’t you get the same sort of problem if the uvloop user tries to do something else with the Python socket object, e.g. call getpeername()? I see the fileno=... parameter for sockets as a parallel to the os.fdopen() function,

[issue26685] Raise errors from socket.close()

Martin Panter added the comment: I think your code example is not very robust, because the “sock” object refers to a freed file descriptor, and could easily close an unrelated file: $ python3.5 -q >>> import socket >>> sock0 = socket.socket() >>> sock = socket.socket(fileno=sock0.fileno()) >>>

[issue26685] Raise errors from socket.close()

Yury Selivanov added the comment: IOW, what is happening in uvloop: 1. A user has a Python socket object and passes it asyncio (run with uvloop) API. 2. uvloop extracts the FD of the socket, and passes it to low-level libuv. 3. At some point of time, libuv closes the FD. 4. When user tries

[issue26685] Raise errors from socket.close()

Yury Selivanov added the comment: I think this patch should be reverted. It breaks backwards compatibility: import socket sock0 = socket.socket(socket.AF_INET, socket.SOCK_STREAM, 0) sock = socket.socket( socket.AF_INET, socket.SOCK_STREAM, 0, sock0.fileno())

[issue26685] Raise errors from socket.close()

Martin Panter added the comment: I tweaked the test again for Windows, anticipating that it will raise ENOTSOCK rather than EBADF. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue26685] Raise errors from socket.close()

Roundup Robot added the comment: New changeset bd665613ed67 by Martin Panter in branch 'default': Issue #26685: Raise OSError if closing a socket fails https://hg.python.org/cpython/rev/bd665613ed67 -- nosy: +python-dev ___ Python tracker

[issue26685] Raise errors from socket.close()

Martin Panter added the comment: Here is socket.close.v2.patch which hopefully fixes the test for Windows (still untested by me though) -- Added file: http://bugs.python.org/file42361/socket.close.v2.patch ___ Python tracker

[issue26685] Raise errors from socket.close()

Martin Panter added the comment: Victor suggested making these errors be reported by the finalize method (garbage collection). I started investigating this, but faced various test suite failures (test_io, test_curses), as well as many new unhandled exceptions printed out during the tests

[issue26685] Raise errors from socket.close()

STINNER Victor added the comment: I like the idea :-) -- nosy: +haypo ___ Python tracker ___ ___

[issue26685] Raise errors from socket.close()

Changes by Martin Panter : -- dependencies: +test_fileno of test_urllibnet intermittently fails ___ Python tracker ___

[issue26685] Raise errors from socket.close()

New submission from Martin Panter: While looking at a recent failure of test_fileno() in test_urllibnet, I discovered that socket.close() ignores the return value of the close() system call. It looks like it has always been this way: