From 68bcf4c4c517897b698450e4d8a17259a7fb57f8 Mon Sep 17 00:00:00 2001 From: "Joshua C. Colp" Date: Tue, 28 Jun 2022 08:59:40 -0300 Subject: [PATCH] websocket / aeap: Handle poll() interruptions better. A sporadic test failure was happening when executing the AEAP Websocket transport tests. It was originally thought this was due to things not getting cleaned up fast enough, but upon further investigation I determined the underlying cause was poll() getting interrupted and this not being handled in all places. This change adds EINTR and EAGAIN handling to the Websocket client connect code as well as the AEAP Websocket transport code. If either occur then the code will just go back to waiting for data. The originally disabled failure test case has also been re-enabled. ASTERISK-30099 Change-Id: I1711a331ecf5d35cd542911dc6aaa9acf1e172ad --- res/res_aeap/transport_websocket.c | 7 ++++++- res/res_http_websocket.c | 20 +++++++++++++++++--- tests/test_aeap_transport.c | 17 +++++------------ 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/res/res_aeap/transport_websocket.c b/res/res_aeap/transport_websocket.c index 5f1a406607..801e4d82a0 100644 --- a/res/res_aeap/transport_websocket.c +++ b/res/res_aeap/transport_websocket.c @@ -103,7 +103,12 @@ static intmax_t websocket_read(struct aeap_transport *self, void *buf, intmax_t * unlock it prior to waiting. */ ast_mutex_unlock(&transport->base.read_lock); - if (ast_websocket_wait_for_input(transport->ws, -1) <= 0) { + while (ast_websocket_wait_for_input(transport->ws, -1) <= 0) { + /* If this was poll getting interrupted just go back to waiting */ + if (errno == EINTR || errno == EAGAIN) { + continue; + } + ast_mutex_lock(&transport->base.read_lock); log_error(self, "poll failure: %s", strerror(errno)); /* Ensure this transport is in a disconnected state */ diff --git a/res/res_http_websocket.c b/res/res_http_websocket.c index d0a5ea72e2..f8da487828 100644 --- a/res/res_http_websocket.c +++ b/res/res_http_websocket.c @@ -1308,7 +1308,11 @@ static enum ast_websocket_result websocket_client_handshake_get_response( int has_accept = 0; int has_protocol = 0; - if (ast_iostream_gets(client->ser->stream, buf, sizeof(buf)) <= 0) { + while (ast_iostream_gets(client->ser->stream, buf, sizeof(buf)) <= 0) { + if (errno == EINTR || errno == EAGAIN) { + continue; + } + ast_log(LOG_ERROR, "Unable to retrieve HTTP status line."); return WS_BAD_STATUS; } @@ -1321,10 +1325,19 @@ static enum ast_websocket_result websocket_client_handshake_get_response( /* Ignoring line folding - assuming header field values are contained within a single line */ - while (ast_iostream_gets(client->ser->stream, buf, sizeof(buf)) > 0) { + while (1) { + ssize_t len = ast_iostream_gets(client->ser->stream, buf, sizeof(buf)); char *name, *value; - int parsed = ast_http_header_parse(buf, &name, &value); + int parsed; + if (len <= 0) { + if (errno == EINTR || errno == EAGAIN) { + continue; + } + break; + } + + parsed = ast_http_header_parse(buf, &name, &value); if (parsed < 0) { break; } @@ -1360,6 +1373,7 @@ static enum ast_websocket_result websocket_client_handshake_get_response( return WS_NOT_SUPPORTED; } } + return has_upgrade && has_connection && has_accept ? WS_OK : WS_HEADER_MISSING; } diff --git a/tests/test_aeap_transport.c b/tests/test_aeap_transport.c index e864d44e6e..cee9c9e958 100644 --- a/tests/test_aeap_transport.c +++ b/tests/test_aeap_transport.c @@ -134,22 +134,15 @@ AST_TEST_DEFINE(transport_connect_fail) ast_test_validate(test, !aeap_transport_is_connected(transport)); - /* - * The following section of code has been disabled as it may be the cause - * of subsequent test failures. - * - * See ASTERISK-30099 for more information - */ - - /* aeap_transport_destroy(transport); */ + aeap_transport_destroy(transport); /* /\* Test invalid protocol *\/ */ - /* ast_test_validate(test, (transport = aeap_transport_create(TRANSPORT_URL))); */ + ast_test_validate(test, (transport = aeap_transport_create(TRANSPORT_URL))); - /* ast_test_validate(test, aeap_transport_connect(transport, */ - /* TRANSPORT_URL, TRANSPORT_PROTOCOL_INVALID, TRANSPORT_TIMEOUT)); */ + ast_test_validate(test, aeap_transport_connect(transport, + TRANSPORT_URL, TRANSPORT_PROTOCOL_INVALID, TRANSPORT_TIMEOUT)); - /* ast_test_validate(test, !aeap_transport_is_connected(transport)); */ + ast_test_validate(test, !aeap_transport_is_connected(transport)); return AST_TEST_PASS; }