From cc25ca6cc2fb996b40ff32c78c8c4dc2a6d5197c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= Date: Fri, 22 Apr 2016 16:16:01 +0200 Subject: [PATCH] better cleanup in http client --- esphttpclient/httpclient.c | 81 +++++++++++++++++++++++++++++++++----- esphttpclient/httpclient.h | 2 + user/user_main.c | 4 +- 3 files changed, 75 insertions(+), 12 deletions(-) diff --git a/esphttpclient/httpclient.c b/esphttpclient/httpclient.c index 38b3595..5cb7afe 100644 --- a/esphttpclient/httpclient.c +++ b/esphttpclient/httpclient.c @@ -25,6 +25,8 @@ typedef struct { int buffer_size; bool secure; httpclient_cb user_callback; + int timeout; + os_timer_t timeout_timer; } request_args; static char *FLASH_FN esp_strdup(const char *str) @@ -224,7 +226,7 @@ static void FLASH_FN sent_callback(void * arg) } else { // The headers were sent, now send the contents. //dbg("Sending request body"); - if (req->secure){ + if (req->secure) { #ifdef USE_SECURE espconn_secure_sent(conn, (uint8_t *)req->post_data, strlen(req->post_data)); #endif @@ -253,6 +255,11 @@ static void FLASH_FN connect_callback(void * arg) sprintf(post_headers, "Content-Length: %d\r\n", strlen(req->post_data)); } + if (req->headers == NULL) { /* Avoid NULL pointer, it may cause exception */ + req->headers = (char *)malloc(sizeof(char)); + req->headers[0] = '\0'; + } + char buf[69 + strlen(method) + strlen(req->path) + strlen(req->hostname) + strlen(req->headers) + strlen(post_headers)]; int len = sprintf(buf, @@ -273,8 +280,27 @@ static void FLASH_FN connect_callback(void * arg) } else { espconn_sent(conn, (uint8_t *)buf, len); } - free(req->headers); - req->headers = NULL; + + if (req->headers != NULL) { + free(req->headers); + req->headers = NULL; + } +} + +/** + * @brief Free all that could be allocated in a request, including the struct itself. + * @param req : req to free + */ +static void free_req(request_args *req) +{ + if (!req) return; + + if (req->buffer) free(req->buffer); + if (req->hostname) free(req->hostname); + if (req->path) free(req->path); + if (req->post_data) free(req->post_data); + if (req->headers) free(req->headers); + free(req); } static void FLASH_FN disconnect_callback(void * arg) @@ -288,9 +314,13 @@ static void FLASH_FN disconnect_callback(void * arg) if (conn->reserve != NULL) { request_args * req = (request_args *)conn->reserve; - int http_status = -1; + int http_status = HTTP_STATUS_GENERIC_ERROR; int body_size = 0; char *body = ""; + + /* Turn off timeout timer */ + os_timer_disarm(&(req->timeout_timer)); + if (req->buffer == NULL) { error("Buffer shouldn't be NULL"); } else if (req->buffer[0] != '\0') { @@ -320,10 +350,7 @@ static void FLASH_FN disconnect_callback(void * arg) req->user_callback(body, http_status, req->buffer, body_size); } - free(req->buffer); - free(req->hostname); - free(req->path); - free(req); + free_req(req); } espconn_delete(conn); if (conn->proto.tcp != NULL) { @@ -338,6 +365,36 @@ static void FLASH_FN error_callback(void *arg, sint8 errType) disconnect_callback(arg); } +static void FLASH_FN http_timeout_callback(void *arg) +{ + error("Connection timeout\n"); + struct espconn * conn = (struct espconn *) arg; + + if (conn == NULL) { + return; + } + + request_args *req = (request_args*) conn->reserve; + if (req) { + /* Call disconnect */ + if (req->secure) { +#ifdef USE_SECURE + espconn_secure_disconnect(conn); +#endif + } else { + espconn_disconnect(conn); + } + + free_req(req); + } + + // experimental - better cleanup + if (conn->proto.tcp != NULL) { + free(conn->proto.tcp); + } + free(conn); +} + static void FLASH_FN dns_callback(const char *hostname, ip_addr_t *addr, void *arg) { request_args * req = (request_args *)arg; @@ -365,6 +422,11 @@ static void FLASH_FN dns_callback(const char *hostname, ip_addr_t *addr, void *a espconn_regist_disconcb(conn, disconnect_callback); espconn_regist_reconcb(conn, error_callback); + /* Set connection timeout timer */ + os_timer_disarm(&(req->timeout_timer)); + os_timer_setfn(&(req->timeout_timer), (os_timer_func_t *) http_timeout_callback, conn); + os_timer_arm(&(req->timeout_timer), req->timeout, false); + if (req->secure) { #ifdef USE_SECURE espconn_secure_set_size(ESPCONN_CLIENT, 5120); // set SSL buffer size @@ -391,6 +453,7 @@ void FLASH_FN http_raw_request(const char *hostname, int port, bool secure, cons req->buffer = (char *)malloc(1); req->buffer[0] = '\0'; // Empty string. req->user_callback = user_callback; + req->timeout = HTTP_REQUEST_TIMEOUT_MS; ip_addr_t addr; err_t error = espconn_gethostbyname((struct espconn *)req, // It seems we don't need a real espconn pointer here. @@ -478,7 +541,7 @@ void FLASH_FN http_get(const char *url, const char *headers, httpclient_cb user_ void FLASH_FN http_callback_example(char *response_body, int http_status, char *response_headers, int body_size) { - dbg("[HTTPCLIENT] http_status=%d", http_status); + dbg("http_status=%d", http_status); if (http_status != HTTP_STATUS_GENERIC_ERROR) { dbg("strlen(headers)=%d", (int)strlen(response_headers)); dbg("body_size=%d", body_size); diff --git a/esphttpclient/httpclient.h b/esphttpclient/httpclient.h index 5afc2fa..4871656 100644 --- a/esphttpclient/httpclient.h +++ b/esphttpclient/httpclient.h @@ -15,6 +15,8 @@ #define HTTP_STATUS_GENERIC_ERROR -1 // In case of TCP or DNS error the callback is called with this status. #define BUFFER_SIZE_MAX 5000 // Size of http responses that will cause an error. +#define HTTP_REQUEST_TIMEOUT_MS 10000 + /* Define this if ssl is needed. Also link the ssl lib */ //#define USE_SECURE diff --git a/user/user_main.c b/user/user_main.c index 2755a3d..7f02ac1 100644 --- a/user/user_main.c +++ b/user/user_main.c @@ -50,9 +50,7 @@ static void ICACHE_FLASH_ATTR prSecondTimerCb(void *arg) cnt2 = 0; dbg("=> Simple GET"); - error("=> Simple GET"); - warn("=> Simple GET"); - info("=> Simple GET"); + //http_get("http://data.ondrovo.com/f/hello.txt", "", http_callback_example); http_get("http://data.ondrovo.com/f/hello.txt", "", http_callback_example); }