Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tcp_connect does not respect the timeout parameter (IDFGH-2971) #5004

Closed
X-Ryl669 opened this issue Mar 26, 2020 · 3 comments
Closed

tcp_connect does not respect the timeout parameter (IDFGH-2971) #5004

X-Ryl669 opened this issue Mar 26, 2020 · 3 comments

Comments

@X-Ryl669
Copy link
Contributor

In current repository, tcp_connect in esp_transport has this signature:

static int tcp_connect(esp_transport_handle_t t, const char *host, int port, int timeout_ms)

Yet, inside the function, there are 2 blocking functions being called and none will respect the timeout_ms given.
The first one is inet_pton/resolve_dns and the second one is connect.
Please notice that lwip does not respect the socket option SO_SNDTIMEO for connect.

As a result, if a server takes forever to connect (for example, by being down), tcp_connect will block indefinitively.

The correct way to have a timeout here is to set the socket as non blocking (via fcntl), call connect, then select for socket writeability within the given timeout.

@github-actions github-actions bot changed the title tcp_connect does not respect the timeout parameter tcp_connect does not respect the timeout parameter (IDFGH-2971) Mar 26, 2020
@X-Ryl669
Copy link
Contributor Author

X-Ryl669 commented Mar 26, 2020

Also, as a more general remark, the parameter timeout_ms in all those transport functions should be called inactivity_time_ms instead since the timeout option set on a socket is reset on socket activity. So if you're receiving a byte per second with a timeout_ms set to 2s, and you are receiving 5 bytes, it'll actually take 5 seconds to complete.

@anecdata
Copy link

anecdata commented Apr 18, 2020

We've noticed that TCP sockets seem to block if, for example, an unreachable numeric IPv4 address is supplied. Sounds like this may be the underlying issue.

@X-Ryl669
Copy link
Contributor Author

X-Ryl669 commented May 8, 2020

You've resolved one issue but not the other. resolve_dns is still blocking.
To resolve a DNS name asynchronously, you should use LWIP's dns_gethostbyname and a semaphore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants