[issue23863] Fix EINTR Socket Module issues in 2.7

STINNER Victor added the comment: I am closing the issue as WONTFIX for all the reasons listen in my previous comments. -- resolution: -> wont fix stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue23863] Fix EINTR Socket Module issues in 2.7

Gregory P. Smith added the comment: Unassigning because i'm unlikely to get to this in 2.7, it is better to concentrate on ensuring that 3.x stays signal safe. -- assignee: gregory.p.smith -> ___ Python tracker

[issue23863] Fix EINTR Socket Module issues in 2.7

Changes by Robert Cope : -- nosy: +rpcope1 ___ Python tracker ___ ___ Python-bugs-list

[issue23863] Fix EINTR Socket Module issues in 2.7

Jeff McNeil added the comment: Do we have a decision on this yet? I'm willing to rework bits that may need it, but I'd like to know whether this is going to be a fruitful effort or not. Thanks! -- ___ Python tracker rep...@bugs.python.org

[issue23863] Fix EINTR Socket Module issues in 2.7

STINNER Victor added the comment: diverging discussion: Go re-read the documentation on os.times(). We don't have the same definition of the unit seconds :-) print(os.times()); time.sleep(1); print(os.times()) (0.04, 0.01, 0.0, 0.0, 4731691.68) (0.04, 0.01, 0.0, 0.0, 4731692.68) My watch

[issue23863] Fix EINTR Socket Module issues in 2.7

STINNER Victor added the comment: We don't have the same definition of the unit seconds :-) Or, please forget my comment, I watched the first 4 items of os.times(). I didn't notice the difference of the 5th item, os.times()[4]. The bad news is that only the first two items of os.times() are

[issue23863] Fix EINTR Socket Module issues in 2.7

Gregory P. Smith added the comment: diverging discussion: Go re-read the documentation on os.times(). It is plural, it isn't just CPU time. (on POSIX os.times()[4] is likely to be the system uptime in seconds as a float... it cannot be changed like the absolute clock can, it is a relative

[issue23863] Fix EINTR Socket Module issues in 2.7

STINNER Victor added the comment: On POSIX systems using os.times()[4] rather than an absolute time.time(), This is just wrong. os.times() doesn't use seconds, but an CPU time which is unrelated. socket.settimeout() uses seconds. By the way, as I wrote before: using the system clock here may

[issue23863] Fix EINTR Socket Module issues in 2.7

Jeff McNeil added the comment: Fixing the underlying connect call should also address EINTR failing with a operation already in progress when a connect+timeout fails due to a signal. I can understand not addressing EINTR generically (though it is already partially addressed in 2.7's

[issue23863] Fix EINTR Socket Module issues in 2.7

Gregory P. Smith added the comment: ... Code review: In socket_eintr.5.patch I don't think the thread safety concerns about the s.settimeout() calls being done from Python code should be an issue but I'll ponder that for a bit. When you've got a socket s, why would code ever be calling

[issue23863] Fix EINTR Socket Module issues in 2.7

R. David Murray added the comment: Is there a bug being fixed here? I mean other than socket not handling EINTR, where I think we agree that handling it is a feature, given the PEP. -- nosy: +r.david.murray ___ Python tracker rep...@bugs.python.org

[issue23863] Fix EINTR Socket Module issues in 2.7

Changes by koobs koobs.free...@gmail.com: -- nosy: +koobs ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23863 ___ ___ Python-bugs-list mailing

[issue23863] Fix EINTR Socket Module issues in 2.7

STINNER Victor added the comment: See also issue #20611 socket.create_connection() doesn't handle EINTR properly which was closed as duplicated of this issue. I'm not sure that #20611 is a duplicate. -- ___ Python tracker rep...@bugs.python.org

[issue23863] Fix EINTR Socket Module issues in 2.7

STINNER Victor added the comment: @Benjamin: since you are the maintainer of Python 2.7, I would like your opinion on this issue. Python older than 3.5 does not handle EINTR for you: if you register a signal handler for a signal, the signal handler does not raise an exception, and a syscall

[issue23863] Fix EINTR Socket Module issues in 2.7

Jeff McNeil added the comment: I'm not a big fan of the settimeout approach myself (and only did it because it was mentioned as a possible approach). I think the existing implementations of EINTR retry also suffer from the same issue in which the timeout isn't adjusted per iteration (but

[issue23863] Fix EINTR Socket Module issues in 2.7

Jeff McNeil added the comment: Added a flag to allow for at least one run -- I know nothing of non-Linux clock resolution. That should handle that. As for the thread safety of the socket timeouts, yeah, that was why I didn't do that initially, I assumed the suggestion to take that approach

[issue23863] Fix EINTR Socket Module issues in 2.7

Jeff McNeil added the comment: Actually, never mind that suggestion. Now that I think a bit more about it, that actually doesn't do anything since I'd still need to set the updated timeout on the current socket object anyway. Whoops. I'll leave it up to you as to whether we go with an

[issue23863] Fix EINTR Socket Module issues in 2.7

STINNER Victor added the comment: With socket_eintr.4.patch, socket methods become non-thread safe. I'm not sure that it's a serious issue, but you have to consider this side effect of your change. (Modify socketmodule.c instead would allow to handle EINTR, recompute timeout and keep the

[issue23863] Fix EINTR Socket Module issues in 2.7

Changes by STINNER Victor victor.stin...@gmail.com: Added file: http://bugs.python.org/file38874/test_connect_eintr4.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23863 ___

[issue23863] Fix EINTR Socket Module issues in 2.7

STINNER Victor added the comment: Ok, since you look to want to fix Python 2.7, I can help you to handle EINTR in socket.connect() since I fixed Python 3.5. If Python 2.7 is fixed, Python 3.4 should also be fixed. connect_eintr-py27.patch: Patch for Python 2.7 to handle EINTR in

[issue23863] Fix EINTR Socket Module issues in 2.7

STINNER Victor added the comment: socket_eintr.2.patch has an issue with timeout. socket_eintr.2.patch retries a socket method when it is interrupted, but it doesn't recompute the timeout. If a program is interrupted every second by a signal, the signal handler doesn't raise an exception and

[issue23863] Fix EINTR Socket Module issues in 2.7

Jeff McNeil added the comment: Updated to recalculate timeout at Python level. The current module already works this way on recv() calls. See attached. I'd be happy to churn through and fix the other modules (using the 3.5 work as a guide), though I think only addressing the higher level

[issue23863] Fix EINTR Socket Module issues in 2.7

STINNER Victor added the comment: I'd be happy to churn through and fix the other modules (using the 3.5 work as a guide), It is risky to modify so much code. The PEP 475 also has an impact on backward compatibility: https://www.python.org/dev/peps/pep-0475/#backward-compatibility IMO it

[issue23863] Fix EINTR Socket Module issues in 2.7

Jeff McNeil added the comment: Missed check on _ex func. -- Added file: http://bugs.python.org/file38865/socket_eintr.2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23863 ___

[issue23863] Fix EINTR Socket Module issues in 2.7

Jeff McNeil added the comment: So, yeah, that's right. In the attached patch, I'm closing the file descriptor if the timeout/error happens on a non-blocking call. It fails with an EBADF on reconnect at that point, but it doesn't potentially leave an FD in the proc's file table. Should be no

[issue23863] Fix EINTR Socket Module issues in 2.7

Gregory P. Smith added the comment: I like the socketmodule.c part of socket_eintr.1.patch, but it appears to still have the issue haypo describes in https://bugs.python.org/issue20611#msg240194 where connect() cannot be called more than once. The kernel carries on with the connect without

[issue23863] Fix EINTR Socket Module issues in 2.7

Changes by Gregory P. Smith g...@krypto.org: -- assignee: - gregory.p.smith keywords: +needs review stage: - patch review type: - behavior ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23863

[issue23863] Fix EINTR Socket Module issues in 2.7

Jeff McNeil added the comment: Whoops. Accidentally attached the wrong patch that I generated during testing. -- Added file: http://bugs.python.org/file38832/socket_eintr.1.patch ___ Python tracker rep...@bugs.python.org

[issue23863] Fix EINTR Socket Module issues in 2.7

Gregory P. Smith added the comment: You may not be, but I am. :). Jeff is aware of PEP 475. Thanks for the awesome work on the real cleanup of this stuff in 3.5. Sanity at last. -- ___ Python tracker rep...@bugs.python.org

[issue23863] Fix EINTR Socket Module issues in 2.7

STINNER Victor added the comment: I have a very good news for you: this issue and more generally all EINTR issues will be solved in Python 3.5. See the PEP 475. I'm not really interested to fix Python 2.7. -- ___ Python tracker

[issue23863] Fix EINTR Socket Module issues in 2.7

Changes by Ned Deily n...@acm.org: -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23863 ___ ___ Python-bugs-list mailing list

[issue23863] Fix EINTR Socket Module issues in 2.7

Jeff McNeil added the comment: mcjeff@mcjeff:~/cpython_clean$ hg summary parent: 95416:fe34dfea16b0 Escaped backslashes in docstrings. branch: 2.7 commit: 3 modified, 3 unknown update: (current) -- keywords: +patch nosy: +gregory.p.smith Added file:

[issue23863] Fix EINTR Socket Module issues in 2.7

New submission from Jeff McNeil: There are a collection of places in the socket module that do not correctly retry on EINTR. Updated to wrap those calls in a retry loop. However, when fixing connect calls, I noticed that when EINTR is retried on a socket with a timeout specified, the retry