diff --git a/components/esp-tls/esp_tls.c b/components/esp-tls/esp_tls.c index 7e6bc8114..d28a06c87 100644 --- a/components/esp-tls/esp_tls.c +++ b/components/esp-tls/esp_tls.c @@ -96,15 +96,23 @@ static ssize_t tcp_write(esp_tls_t *tls, const char *data, size_t datalen) * @brief Close the TLS connection and free any allocated resources. */ void esp_tls_conn_delete(esp_tls_t *tls) +{ + esp_tls_conn_destroy(tls); +} + +int esp_tls_conn_destroy(esp_tls_t *tls) { if (tls != NULL) { + int ret = 0; _esp_tls_conn_delete(tls); if (tls->sockfd >= 0) { - close(tls->sockfd); + ret = close(tls->sockfd); } - free(tls->error_handle); - free(tls); + free(tls->error_handle); + free(tls); + return ret; } + return -1; // invalid argument } esp_tls_t *esp_tls_init(void) diff --git a/components/esp-tls/esp_tls.h b/components/esp-tls/esp_tls.h index 5d982d3dc..f2f641b95 100644 --- a/components/esp-tls/esp_tls.h +++ b/components/esp-tls/esp_tls.h @@ -463,6 +463,15 @@ static inline ssize_t esp_tls_conn_read(esp_tls_t *tls, void *data, size_t data return tls->read(tls, (char *)data, datalen); } +/** + * @brief Compatible version of esp_tls_conn_destroy() to close the TLS/SSL connection + * + * @note This API will be removed in IDFv5.0 + * + * @param[in] tls pointer to esp-tls as esp-tls handle. + */ +void esp_tls_conn_delete(esp_tls_t *tls); + /** * @brief Close the TLS/SSL connection and free any allocated resources. * @@ -470,8 +479,11 @@ static inline ssize_t esp_tls_conn_read(esp_tls_t *tls, void *data, size_t data * esp_tls_conn_http_new() APIs. * * @param[in] tls pointer to esp-tls as esp-tls handle. + * + * @return - 0 on success + * - -1 if socket error or an invalid argument */ -void esp_tls_conn_delete(esp_tls_t *tls); +int esp_tls_conn_destroy(esp_tls_t *tls); /** * @brief Return the number of application data bytes remaining to be diff --git a/components/tcp_transport/test/test_transport.c b/components/tcp_transport/test/test_transport.c index 28702fb08..8a52577b7 100644 --- a/components/tcp_transport/test/test_transport.c +++ b/components/tcp_transport/test/test_transport.c @@ -4,6 +4,111 @@ #include "esp_transport_tcp.h" #include "esp_transport_ssl.h" #include "esp_transport_ws.h" +#include "test_utils.h" +#include "esp_log.h" +#include "lwip/err.h" +#include "lwip/sockets.h" +#include "lwip/sys.h" +#include +#include "freertos/event_groups.h" + +#define TCP_CONNECT_DONE (1) +#define TCP_LISTENER_DONE (2) + +struct tcp_connect_task_params { + int timeout_ms; + int port; + EventGroupHandle_t tcp_connect_done; + int ret; + int listen_sock; + int last_connect_sock; + bool tcp_listener_failed; +}; + +/** + * @brief Recursively connects with a new socket to loopback interface until the last one blocks. + * The last socket is closed upon test teardown, that initiates recursive cleanup (close) for all + * active/connected sockets. + */ +static void connect_once(struct tcp_connect_task_params *params) +{ + struct sockaddr_in dest_addr_ip4 = { .sin_addr.s_addr = htonl(INADDR_LOOPBACK), + .sin_family = AF_INET, + .sin_port = htons(params->port) }; + int connect_sock = socket(AF_INET, SOCK_STREAM, IPPROTO_IP); + if (connect_sock < 0) { + params->tcp_listener_failed = true; + return; + } + params->last_connect_sock = connect_sock; + int err = connect(connect_sock, (struct sockaddr *)&dest_addr_ip4, sizeof(dest_addr_ip4)); + if (err != 0) { + // The last connection is expected to fail here, since the both sockets get closed on test cleanup + return; + } + connect_once(params); + close(connect_sock); +} + +/** + * @brief creates a listener (without and acceptor) and connect to as many times as possible + * to prepare an endpoint which would make the client block but not complete TCP handshake + */ +static void localhost_listener(void *pvParameters) +{ + const char* TAG = "tcp_transport_test"; + struct tcp_connect_task_params *params = pvParameters; + struct sockaddr_in dest_addr_ip4 = { .sin_addr.s_addr = htonl(INADDR_ANY), + .sin_family = AF_INET, + .sin_port = htons(params->port) }; + // Create listener socket and bind it to ANY address + params->listen_sock = socket(AF_INET, SOCK_STREAM, IPPROTO_IP); + if (params->listen_sock < 0) { + ESP_LOGE(TAG, "Unable to create socket"); + params->tcp_listener_failed = true; + goto failed; + } + int err = bind(params->listen_sock, (struct sockaddr *)&dest_addr_ip4, sizeof(dest_addr_ip4)); + if (err != 0) { + ESP_LOGE(TAG, "Socket unable to bind: errno %d", errno); + params->tcp_listener_failed = true; + goto failed; + } + + // Listen with backlog set to a low number + err = listen(params->listen_sock, 4); + if (err != 0) { + ESP_LOGE(TAG, "Error occurred during listen: errno %d", errno); + params->tcp_listener_failed = true; + goto failed; + } + + // Ideally we would set backlog to 0, but since this is an implementation specific recommendation parameter, + // we recursively create sockets and try to connect to this listener in order to consume the backlog. After + // the backlog is consumed, the last connection blocks (waiting for accept), but at that point we are sure + // that any other connection would also block + connect_once(params); + +failed: + xEventGroupSetBits(params->tcp_connect_done, TCP_LISTENER_DONE); + vTaskSuspend(NULL); +} + +static void tcp_connect_task(void *pvParameters) +{ + struct tcp_connect_task_params *params = pvParameters; + + esp_transport_list_handle_t transport_list = esp_transport_list_init(); + esp_transport_handle_t tcp = esp_transport_tcp_init(); + esp_transport_list_add(transport_list, tcp, "tcp"); + + params->ret = esp_transport_connect(tcp, "localhost", params->port, params->timeout_ms); + xEventGroupSetBits(params->tcp_connect_done, TCP_CONNECT_DONE); + esp_transport_close(tcp); + esp_transport_list_destroy(transport_list); + vTaskSuspend(NULL); +} + TEST_CASE("tcp_transport: init and deinit transport list", "[tcp_transport][leaks=0]") { @@ -26,3 +131,49 @@ TEST_CASE("tcp_transport: using ws transport separately", "[tcp_transport][leaks TEST_ASSERT_EQUAL(ESP_OK, esp_transport_destroy(ws)); TEST_ASSERT_EQUAL(ESP_OK, esp_transport_destroy(tcp)); } + +TEST_CASE("tcp_transport: connect timeout", "[tcp_transport]") +{ + // This case simulates connection timeout running tcp connect asynchronously with other socket connection + // consuming entire socket listener backlog. + // Important: Both tasks must run on the same core, with listener's prio higher to make sure that + // 1) first the localhost_listener() creates and connects all sockets until the last one blocks + // 2) before the tcp_connect_task() attempts to connect and thus fails with connection timeout + + struct tcp_connect_task_params params = { .tcp_connect_done = xEventGroupCreate(), + .timeout_ms = 200, + .port = 80 }; + TickType_t max_wait = pdMS_TO_TICKS(params.timeout_ms * 10); + TaskHandle_t localhost_listener_task_handle = NULL; + TaskHandle_t tcp_connect_task_handle = NULL; + + test_case_uses_tcpip(); + + // Create listener and connect it with as many sockets until the last one blocks + xTaskCreatePinnedToCore(localhost_listener, "localhost_listener", 4096, (void*)¶ms, 5, &localhost_listener_task_handle, 0); + + // Perform tcp-connect in a separate task to check asynchronously for the timeout + xTaskCreatePinnedToCore(tcp_connect_task, "tcp_connect_task", 4096, (void*)¶ms, 4, &tcp_connect_task_handle, 0); + + // Roughly measure tick-time spent while trying to connect + TickType_t start = xTaskGetTickCount(); + EventBits_t bits = xEventGroupWaitBits(params.tcp_connect_done, TCP_CONNECT_DONE, true, true, max_wait); + TickType_t end = xTaskGetTickCount(); + + TEST_ASSERT_EQUAL(TCP_CONNECT_DONE, bits); // Connection has finished + TEST_ASSERT_EQUAL(-1, params.ret); // Connection failed with -1 + + // Test connection attempt took expected timeout value + TEST_ASSERT_INT_WITHIN(pdMS_TO_TICKS(params.timeout_ms/5), pdMS_TO_TICKS(params.timeout_ms), end-start); + + // Closing both parties of the last "blocking" connection to unwind localhost_listener() and let other connected sockets closed + close(params.listen_sock); + close(params.last_connect_sock); + + // Cleanup + xEventGroupWaitBits(params.tcp_connect_done, TCP_LISTENER_DONE, true, true, max_wait); + TEST_ASSERT_EQUAL(false, params.tcp_listener_failed); + vEventGroupDelete(params.tcp_connect_done); + test_utils_task_delete(localhost_listener_task_handle); + test_utils_task_delete(tcp_connect_task_handle); +} diff --git a/components/tcp_transport/transport_ssl.c b/components/tcp_transport/transport_ssl.c index 23c2b0944..98bda19a0 100644 --- a/components/tcp_transport/transport_ssl.c +++ b/components/tcp_transport/transport_ssl.c @@ -74,7 +74,7 @@ static int ssl_connect(esp_transport_handle_t t, const char *host, int port, int if (esp_tls_conn_new_sync(host, strlen(host), port, &ssl->cfg, ssl->tls) <= 0) { ESP_LOGE(TAG, "Failed to open a new connection"); esp_transport_set_errors(t, ssl->tls->error_handle); - esp_tls_conn_delete(ssl->tls); + esp_tls_conn_destroy(ssl->tls); ssl->tls = NULL; return -1; } @@ -170,7 +170,7 @@ static int ssl_close(esp_transport_handle_t t) int ret = -1; transport_ssl_t *ssl = esp_transport_get_context_data(t); if (ssl->ssl_initialized) { - esp_tls_conn_delete(ssl->tls); + ret = esp_tls_conn_destroy(ssl->tls); ssl->conn_state = TRANS_SSL_INIT; ssl->ssl_initialized = false; } diff --git a/components/tcp_transport/transport_tcp.c b/components/tcp_transport/transport_tcp.c index 975d0e874..69137ffca 100644 --- a/components/tcp_transport/transport_tcp.c +++ b/components/tcp_transport/transport_tcp.c @@ -81,14 +81,68 @@ static int tcp_connect(esp_transport_handle_t t, const char *host, int port, int setsockopt(tcp->sock, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); setsockopt(tcp->sock, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)); - ESP_LOGD(TAG, "[sock=%d],connecting to server IP:%s,Port:%d...", - tcp->sock, ipaddr_ntoa((const ip_addr_t*)&remote_ip.sin_addr.s_addr), port); - if (connect(tcp->sock, (struct sockaddr *)(&remote_ip), sizeof(struct sockaddr)) != 0) { - close(tcp->sock); - tcp->sock = -1; - return -1; + // Set socket to non-blocking + int flags; + if ((flags = fcntl(tcp->sock, F_GETFL, NULL)) < 0) { + ESP_LOGE(TAG, "[sock=%d] get file flags error: %s", tcp->sock, strerror(errno)); + goto error; + } + if (fcntl(tcp->sock, F_SETFL, flags |= O_NONBLOCK) < 0) { + ESP_LOGE(TAG, "[sock=%d] set nonblocking error: %s", tcp->sock, strerror(errno)); + goto error; + } + + ESP_LOGD(TAG, "[sock=%d] Connecting to server. IP: %s, Port: %d", + tcp->sock, ipaddr_ntoa((const ip_addr_t*)&remote_ip.sin_addr.s_addr), port); + + if (connect(tcp->sock, (struct sockaddr *)(&remote_ip), sizeof(struct sockaddr)) < 0) { + if (errno == EINPROGRESS) { + fd_set fdset; + + esp_transport_utils_ms_to_timeval(timeout_ms, &tv); + FD_ZERO(&fdset); + FD_SET(tcp->sock, &fdset); + + int res = select(tcp->sock+1, NULL, &fdset, NULL, &tv); + if (res < 0) { + ESP_LOGE(TAG, "[sock=%d] select() error: %s", tcp->sock, strerror(errno)); + goto error; + } + else if (res == 0) { + ESP_LOGE(TAG, "[sock=%d] select() timeout", tcp->sock); + goto error; + } else { + int sockerr; + socklen_t len = (socklen_t)sizeof(int); + + if (getsockopt(tcp->sock, SOL_SOCKET, SO_ERROR, (void*)(&sockerr), &len) < 0) { + ESP_LOGE(TAG, "[sock=%d] getsockopt() error: %s", tcp->sock, strerror(errno)); + goto error; + } + else if (sockerr) { + ESP_LOGE(TAG, "[sock=%d] delayed connect error: %s", tcp->sock, strerror(sockerr)); + goto error; + } + } + } else { + ESP_LOGE(TAG, "[sock=%d] connect() error: %s", tcp->sock, strerror(errno)); + goto error; + } + } + // Reset socket to blocking + if ((flags = fcntl(tcp->sock, F_GETFL, NULL)) < 0) { + ESP_LOGE(TAG, "[sock=%d] get file flags error: %s", tcp->sock, strerror(errno)); + goto error; + } + if (fcntl(tcp->sock, F_SETFL, flags & ~O_NONBLOCK) < 0) { + ESP_LOGE(TAG, "[sock=%d] reset blocking error: %s", tcp->sock, strerror(errno)); + goto error; } return tcp->sock; +error: + close(tcp->sock); + tcp->sock = -1; + return -1; } static int tcp_write(esp_transport_handle_t t, const char *buffer, int len, int timeout_ms)