| msg117385 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-09-25 19:07 |
| This was introduced by r74426 which addressed . socket.sendall() calls PyErr_CheckSignals() (and potentially returns to the caller) without having the GIL. >>> import socket >>> c, s = socket.socketpair() >>> s.sendall(b"x"*(100 * 1024**2)) ^C^CFatal Python error: PyThreadState_Get: no current thread |
|
|
| msg117387 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-09-25 19:11 |
| The fix is very simple, but perhaps a test should be added. diff -r af0d7b32d6ce Modules/socketmodule.c --- a/Modules/socketmodule.c Fri Sep 24 20:03:12 2010 +0200 +++ b/Modules/socketmodule.c Sat Sep 25 21:09:58 2010 +0200 @@ -2581,8 +2581,8 @@ sock_sendall(PySocketSockObject *s, PyOb return select_error(); } - Py_BEGIN_ALLOW_THREADS do { + Py_BEGIN_ALLOW_THREADS timeout = internal_select(s, 1); n = -1; if (timeout) @@ -2592,6 +2592,7 @@ sock_sendall(PySocketSockObject *s, PyOb #else n = send(s->sock_fd, buf, len, flags); #endif + Py_END_ALLOW_THREADS if (n < 0) { #ifdef EINTR /* We must handle EINTR here as there is no way for @@ -2610,7 +2611,6 @@ sock_sendall(PySocketSockObject *s, PyOb buf += n; len -= n; } while (len > 0); - Py_END_ALLOW_THREADS PyBuffer_Release(&pbuf); if (timeout == 1) { |
|
|
| msg117388 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-09-25 19:26 |
| Actually, the patch is enough to suppress the crash, but sendall() behaviour is buggy in another way: EINTR may be received as part of the select() call (on sockets with a timeout), in which case the loop will be exited early instead of retrying, losing track of the number of bytes written (something which the original patch aimed to avoid). For example: >>> def handler(*args): print (args) ... >>> signal.signal(signal.SIGALRM, handler) 0 >>> c, s = socket.socketpair() >>> s.settimeout(60.0) >>> signal.alarm(1); s.sendall(b"x" * (100 * 1024**2)) 0 (14, <frame object at 0x2b8f220>) Traceback (most recent call last): File "", line 1, in socket.error: [Errno 4] Interrupted system call |
|
|
| msg117389 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-09-25 19:54 |
| Oh, and an additional bug is that send() can return a successful partial write when it was actually interrupted by a signal. |
|
|
| msg117390 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-09-25 20:27 |
| Here is a patch fixing the aforementioned issues, and with tests. |
|
|
| msg117453 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-09-27 17:56 |
| Patch committed in r85032. I'm gonna watch the buildbots a bit, in case the test fails on some platforms. |
|
|
| msg117460 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2010-09-27 18:33 |
| The patch passes at least on Linux, OS X and Solaris buildbots. Backported to 3.1 (r85034) and 2.7 (r85035). |
|
|
| msg117505 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2010-09-28 05:59 |
| thanks Antoine! |
|
|