Skip to content

Commit df0d7e9

Browse files
committed
extmod/modlwip: Make socket.connect raise ETIMEDOUT on non-zero timeout.
If the socket timeout is 0 then a failed socket.connect() raises EINPROGRESS (which is what the lwIP bindings already did), but if the socket timeout is non-zero then a failed socket.connect() should raise ETIMEDOUT. The latter is fixed in this commit. A test is added for these timeout cases. Signed-off-by: Damien George <[email protected]>
1 parent 80a4f63 commit df0d7e9

File tree

2 files changed

+40
-12
lines changed

2 files changed

+40
-12
lines changed

extmod/modlwip.c

+16-12
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,10 @@ typedef struct _lwip_socket_obj_t {
326326
int8_t state;
327327
} lwip_socket_obj_t;
328328

329+
static inline bool socket_is_timedout(lwip_socket_obj_t *socket, mp_uint_t ticks_start) {
330+
return socket->timeout != -1 && (mp_uint_t)(mp_hal_ticks_ms() - ticks_start) >= socket->timeout;
331+
}
332+
329333
static inline void poll_sockets(void) {
330334
mp_event_wait_ms(1);
331335
}
@@ -1130,21 +1134,21 @@ static mp_obj_t lwip_socket_connect(mp_obj_t self_in, mp_obj_t addr_in) {
11301134
MICROPY_PY_LWIP_EXIT
11311135

11321136
// And now we wait...
1133-
if (socket->timeout != -1) {
1134-
for (mp_uint_t retries = socket->timeout / 100; retries--;) {
1135-
mp_hal_delay_ms(100);
1136-
if (socket->state != STATE_CONNECTING) {
1137-
break;
1138-
}
1139-
}
1140-
if (socket->state == STATE_CONNECTING) {
1141-
mp_raise_OSError(MP_EINPROGRESS);
1137+
mp_uint_t ticks_start = mp_hal_ticks_ms();
1138+
for (;;) {
1139+
poll_sockets();
1140+
if (socket->state != STATE_CONNECTING) {
1141+
break;
11421142
}
1143-
} else {
1144-
while (socket->state == STATE_CONNECTING) {
1145-
poll_sockets();
1143+
if (socket_is_timedout(socket, ticks_start)) {
1144+
if (socket->timeout == 0) {
1145+
mp_raise_OSError(MP_EINPROGRESS);
1146+
} else {
1147+
mp_raise_OSError(MP_ETIMEDOUT);
1148+
}
11461149
}
11471150
}
1151+
11481152
if (socket->state == STATE_CONNECTED) {
11491153
err = ERR_OK;
11501154
} else {

tests/net_hosted/connect_timeout.py

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# Test that socket.connect() on a socket with timeout raises EINPROGRESS or ETIMEDOUT appropriately.
2+
3+
import errno
4+
import socket
5+
6+
7+
def test(peer_addr, timeout, expected_exc):
8+
s = socket.socket()
9+
s.settimeout(timeout)
10+
try:
11+
s.connect(peer_addr)
12+
print("OK")
13+
except OSError as er:
14+
print(er.args[0] in expected_exc)
15+
s.close()
16+
17+
18+
if __name__ == "__main__":
19+
# This test needs an address that doesn't respond to TCP connections.
20+
# 1.1.1.1:8000 seem to reliably timeout, so use that.
21+
addr = socket.getaddrinfo("1.1.1.1", 8000)[0][-1]
22+
23+
test(addr, 0, (errno.EINPROGRESS,))
24+
test(addr, 1, (errno.ETIMEDOUT, "timed out")) # CPython uses a string instead of errno

0 commit comments

Comments
 (0)