michaelh/zephyr
1
0
Fork 0
Code Releases Activity

net: http: Use poll for http timeout instead of shutdown

Browse source 
Change the http timeout mechanism to use poll instead of shutdown.
This should fix a problem where the shutdown will be called in a
different thread context which can lead to deadlocks on certain
driver implementations like offloaded modem drivers.
Fixes #53967

Signed-off-by: Wouter Cappelle <wouter.cappelle@crodeon.com>
This commit is contained in:
Wouter Cappelle 2023-01-23 11:35:47 +01:00 • committed by Carles Cufí
commit aaefd3db7e
2 changed files with 106 additions and 101 deletions
Download patch file Download diff file Expand all files Collapse all files

6
include/zephyr/net/http/client.h
View file

@ -199,9 +199,6 @@ struct http_response {
/** HTTP client internal data that the application should not touch /** HTTP client internal data that the application should not touch
*/ */
struct http_client_internal_data { struct http_client_internal_data {
/** Work for handling timeout */
struct k_work_delayable work;
/** HTTP parser context */ /** HTTP parser context */
struct http_parser parser; struct http_parser parser;
@ -218,9 +215,6 @@ struct http_client_internal_data {
/** HTTP socket */ /** HTTP socket */
int sock; int sock;
/** Request timeout */
k_timeout_t timeout;
}; };
/** /**

201
subsys/net/lib/http/http_client.c
View file

@ -392,111 +392,135 @@ static void http_client_init_parser(struct http_parser *parser,
settings->on_url = on_url; settings->on_url = on_url;
} }
static int http_wait_data(int sock, struct http_request *req) static void http_data_final_null_resp(struct http_request *req)
{
if (req->internal.response.cb) {
NET_DBG("Calling callback for Final Data"
"(NULL HTTP response)");
/* Status code 0 representing a null response */
req->internal.response.http_status_code = 0;
/* Zero out related response metrics */
req->internal.response.processed = 0;
req->internal.response.data_len = 0;
req->internal.response.content_length = 0;
req->internal.response.body_frag_start = NULL;
memset(req->internal.response.http_status, 0, HTTP_STATUS_STR_SIZE);
req->internal.response.cb(&req->internal.response, HTTP_DATA_FINAL,
req->internal.user_data);
}
}
static int http_wait_data(int sock, struct http_request *req, int32_t timeout)
{ {
int total_received = 0; int total_received = 0;
size_t offset = 0; size_t offset = 0;
int received, ret; int received, ret;
struct zsock_pollfd fds[1];
int nfds = 1;
int32_t remaining_time = timeout;
int64_t timestamp = k_uptime_get();
fds[0].fd = sock;
fds[0].events = POLLIN;
do { do {
received = zsock_recv(sock, req->internal.response.recv_buf + offset, if (timeout > 0) {
req->internal.response.recv_buf_len - offset, remaining_time -= (int32_t)k_uptime_delta(&timestamp);
0); if (remaining_time < 0) {
if (received == 0) { /* timeout, make poll return immediately */
remaining_time = 0;
}
}
ret = zsock_poll(fds, nfds, remaining_time);
if (ret == 0) {
LOG_DBG("Timeout");
goto finalize_data;
} else if (ret < 0) {
goto error;
}
if (fds[0].revents & (ZSOCK_POLLERR | ZSOCK_POLLNVAL)) {
goto error;
} else if (fds[0].revents & ZSOCK_POLLHUP) {
/* Connection closed */ /* Connection closed */
LOG_DBG("Connection closed"); LOG_DBG("Connection closed");
ret = total_received; goto finalize_data;
} else if (fds[0].revents & ZSOCK_POLLIN) {
received = zsock_recv(sock, req->internal.response.recv_buf + offset,
req->internal.response.recv_buf_len - offset, 0);
if (received == 0) {
/* Connection closed */
LOG_DBG("Connection closed");
goto finalize_data;
} else if (received < 0) {
goto error;
} else {
req->internal.response.data_len += received;
(void)http_parser_execute(
&req->internal.parser, &req->internal.parser_settings,
req->internal.response.recv_buf + offset, received);
}
total_received += received;
offset += received;
if (offset >= req->internal.response.recv_buf_len) {
offset = 0;
}
if (req->internal.response.cb) { if (req->internal.response.cb) {
NET_DBG("Calling callback for closed connection " bool notify = false;
"(NULL HTTP response)"); enum http_final_call event;
/* Status code 0 representing a null response */ if (req->internal.response.message_complete) {
req->internal.response.http_status_code = 0; NET_DBG("Calling callback for %zd len data",
req->internal.response.data_len);
/* Zero out related response metrics */ notify = true;
req->internal.response.processed = 0; event = HTTP_DATA_FINAL;
req->internal.response.data_len = 0; } else if (offset == 0) {
req->internal.response.content_length = 0; NET_DBG("Calling callback for partitioned %zd len data",
req->internal.response.body_frag_start = NULL; req->internal.response.data_len);
memset(req->internal.response.http_status, 0,
HTTP_STATUS_STR_SIZE);
req->internal.response.cb(&req->internal.response, notify = true;
HTTP_DATA_FINAL, event = HTTP_DATA_MORE;
req->internal.user_data); }
if (notify) {
req->internal.response.cb(&req->internal.response, event,
req->internal.user_data);
/* Re-use the result buffer and start to fill it again */
req->internal.response.data_len = 0;
req->internal.response.body_frag_start = NULL;
req->internal.response.body_frag_len = 0;
}
} }
break;
} else if (received < 0) {
/* Socket error */
LOG_DBG("Connection error (%d)", errno);
ret = -errno;
break;
} else {
req->internal.response.data_len += received;
(void)http_parser_execute(
&req->internal.parser,
&req->internal.parser_settings,
req->internal.response.recv_buf + offset,
received);
}
total_received += received;
offset += received;
if (offset >= req->internal.response.recv_buf_len) {
offset = 0;
}
if (req->internal.response.cb) {
bool notify = false;
enum http_final_call event;
if (req->internal.response.message_complete) { if (req->internal.response.message_complete) {
NET_DBG("Calling callback for %zd len data", ret = total_received;
req->internal.response.data_len); break;
notify = true;
event = HTTP_DATA_FINAL;
} else if (offset == 0) {
NET_DBG("Calling callback for partitioned %zd len data",
req->internal.response.data_len);
notify = true;
event = HTTP_DATA_MORE;
} }
if (notify) {
req->internal.response.cb(&req->internal.response,
event,
req->internal.user_data);
/* Re-use the result buffer and start to fill it again */
req->internal.response.data_len = 0;
req->internal.response.body_frag_start = NULL;
req->internal.response.body_frag_len = 0;
}
}
if (req->internal.response.message_complete) {
ret = total_received;
break;
} }
} while (true); } while (true);
return ret; return ret;
}
static void http_timeout(struct k_work *work) finalize_data:
{ ret = total_received;
struct k_work_delayable *dwork = k_work_delayable_from_work(work);
struct http_client_internal_data *data =
CONTAINER_OF(dwork, struct http_client_internal_data, work);
(void)zsock_shutdown(data->sock, ZSOCK_SHUT_RD); http_data_final_null_resp(req);
return ret;
error:
LOG_DBG("Connection error (%d)", errno);
ret = -errno;
return ret;
} }
int http_client_req(int sock, struct http_request *req, int http_client_req(int sock, struct http_request *req,
@ -523,7 +547,6 @@ int http_client_req(int sock, struct http_request *req,
req->internal.response.recv_buf_len = req->recv_buf_len; req->internal.response.recv_buf_len = req->recv_buf_len;
req->internal.user_data = user_data; req->internal.user_data = user_data;
req->internal.sock = sock; req->internal.sock = sock;
req->internal.timeout = SYS_TIMEOUT_MS(timeout);
method = http_method_str(req->method); method = http_method_str(req->method);
@ -690,26 +713,14 @@ int http_client_req(int sock, struct http_request *req,
http_client_init_parser(&req->internal.parser, http_client_init_parser(&req->internal.parser,
&req->internal.parser_settings); &req->internal.parser_settings);
if (!K_TIMEOUT_EQ(req->internal.timeout, K_FOREVER) &&
!K_TIMEOUT_EQ(req->internal.timeout, K_NO_WAIT)) {
k_work_init_delayable(&req->internal.work, http_timeout);
(void)k_work_reschedule(&req->internal.work,
req->internal.timeout);
}
/* Request is sent, now wait data to be received */ /* Request is sent, now wait data to be received */
total_recv = http_wait_data(sock, req); total_recv = http_wait_data(sock, req, timeout);
if (total_recv < 0) { if (total_recv < 0) {
NET_DBG("Wait data failure (%d)", total_recv); NET_DBG("Wait data failure (%d)", total_recv);
} else { } else {
NET_DBG("Received %d bytes", total_recv); NET_DBG("Received %d bytes", total_recv);
} }
if (!K_TIMEOUT_EQ(req->internal.timeout, K_FOREVER) &&
!K_TIMEOUT_EQ(req->internal.timeout, K_NO_WAIT)) {
(void)k_work_cancel_delayable(&req->internal.work);
}
return total_sent; return total_sent;
out: out: