Re: test_asynchat.py broken on Windows
I think there are at least three things wrong, depending on what you count as wrong. The test is hanging because it spawns a thread, which Python will prevent Python from exiting until the thread exits. The thread doesn't exit because it's waiting for input from the client. The client doesn't send it any input because of the exception. thing wrong #1: It would probably make sense to change test_asynchat.py to use a daemon thread so that it can't prevent the test suite from exiting. thing wrong #2: I don't think the ENONET being returned by connect_ex() makes any sense. My guess is that connect_ex() is (and always has been) broken on Windows. Could you apply this patch and see what happens: Index: socketmodule.c =================================================================== RCS file: /cvsroot/python/python/dist/src/Modules/socketmodule.c,v retrieving revision 1.190 diff -c -c -r1.190 socketmodule.c *** socketmodule.c 2001/10/28 12:31:33 1.190 --- socketmodule.c 2001/10/29 23:14:18 *************** *** 1268,1274 **** --- 1268,1278 ---- res = connect(s->sock_fd, addr, addrlen); Py_END_ALLOW_THREADS if (res != 0) + #ifdef MS_WINDOWS + res = WSAGetLastError(); + #else res = errno; + #endif return PyInt_FromLong((long) res); } thing wrong #3: From the winsock documentation for connect(), it looks like there are more errors we should be catching in the test that includes EWOULDBLOCK and EALREADY. I don't think this would affect your test, though. thing wrong #4: It doesn't look to me like the connect() call in test_asynchat.py is guaranteed to succeed. It should succeed almost every time, but there's some small chance it will fail. Perhaps the test should be more robust about that. That's four things, but I don't think the last thing is all that wrong. Jeremy
[Jeremy Hylton]
I think there are at least three things wrong, depending on what you count as wrong.
I would have guessed five, if you were counting Windows too <wink>.
I don't much care about this.
Fixed the problem, so I checked it in. Thanks!
I'm not sure I parsed that as intended; asyncore.dispatcher.connect (in the traceback when test_asynchat failed) is already checking for EINPROGRESS, EALREADY, EWOULDBLOCK, 0 and EISCONN returns from connect_ex. But, for posterity, I will inscribe my socket programming knowledge in the period at the end of this sentence.
FWIW, I've never seen it fail before.
That's four things, but I don't think the last thing is all that wrong.
Here's a fifth: def main(): s = echo_server() s.start() time.sleep(1) # Give server time to initialize c = echo_client() ... time.sleep()-is-not-a-synchronization-gimmick-ly y'rs - tim
not sure jeremy gets mail sent to his sourceforge address, but maybe someone else can explain how his recent patch preserves the original behaviour. after the patch, connect will always raise an exception unless the error code is one of the three listed code. before the patch, it will not raise an exception if the connection succeeds during the connect itself (i.e. if people use asyncore with blocking sockets, or if they're using a hypothetical transport that can do an immediate connect). how about adding an "else:" before that last raise? def connect (self, address): self.connected = 0 ! # XXX why not use connect_ex? ! try: ! self.socket.connect (address) ! except socket.error, why: ! if why[0] in (EINPROGRESS, EALREADY, EWOULDBLOCK): ! return ! else: ! raise socket.error, why ! self.addr = address ! self.connected = 1 ! self.handle_connect() def accept (self): --- 302,313 ---- def connect (self, address): self.connected = 0 ! err = self.socket.connect_ex(address) ! if err in (EINPROGRESS, EALREADY, EWOULDBLOCK): ! return ! if err in (0, EISCONN): ! self.addr = address ! self.connected = 1 ! self.handle_connect() ! raise socket.error, err def accept (self):
[Jeremy Hylton]
I think there are at least three things wrong, depending on what you count as wrong.
I would have guessed five, if you were counting Windows too <wink>.
I don't much care about this.
Fixed the problem, so I checked it in. Thanks!
I'm not sure I parsed that as intended; asyncore.dispatcher.connect (in the traceback when test_asynchat failed) is already checking for EINPROGRESS, EALREADY, EWOULDBLOCK, 0 and EISCONN returns from connect_ex. But, for posterity, I will inscribe my socket programming knowledge in the period at the end of this sentence.
FWIW, I've never seen it fail before.
That's four things, but I don't think the last thing is all that wrong.
Here's a fifth: def main(): s = echo_server() s.start() time.sleep(1) # Give server time to initialize c = echo_client() ... time.sleep()-is-not-a-synchronization-gimmick-ly y'rs - tim
not sure jeremy gets mail sent to his sourceforge address, but maybe someone else can explain how his recent patch preserves the original behaviour. after the patch, connect will always raise an exception unless the error code is one of the three listed code. before the patch, it will not raise an exception if the connection succeeds during the connect itself (i.e. if people use asyncore with blocking sockets, or if they're using a hypothetical transport that can do an immediate connect). how about adding an "else:" before that last raise? def connect (self, address): self.connected = 0 ! # XXX why not use connect_ex? ! try: ! self.socket.connect (address) ! except socket.error, why: ! if why[0] in (EINPROGRESS, EALREADY, EWOULDBLOCK): ! return ! else: ! raise socket.error, why ! self.addr = address ! self.connected = 1 ! self.handle_connect() def accept (self): --- 302,313 ---- def connect (self, address): self.connected = 0 ! err = self.socket.connect_ex(address) ! if err in (EINPROGRESS, EALREADY, EWOULDBLOCK): ! return ! if err in (0, EISCONN): ! self.addr = address ! self.connected = 1 ! self.handle_connect() ! raise socket.error, err def accept (self):
participants (3)
-
Fredrik Lundh -
Jeremy Hylton -
Tim Peters