[Asterisk-code-review] websocket / aeap: Handle poll() interruptions better. (asterisk[19])

[Joshua Colp]

Joshua Colp has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/18719 )


Change subject: websocket / aeap: Handle poll() interruptions better.
......................................................................

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
---
M res/res_aeap/transport_websocket.c
M res/res_http_websocket.c
M tests/test_aeap_transport.c
3 files changed, 28 insertions(+), 16 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/19/18719/1

diff --git a/res/res_aeap/transport_websocket.c b/res/res_aeap/transport_websocket.c
index 5f1a406..801e4d8 100644
--- a/res/res_aeap/transport_websocket.c
+++ b/res/res_aeap/transport_websocket.c
@@ -103,7 +103,12 @@
 	 * 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 d0a5ea7..f8da487 100644
--- a/res/res_http_websocket.c
+++ b/res/res_http_websocket.c
@@ -1308,7 +1308,11 @@
 	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 @@
 
 	/* 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 @@
 			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 e864d44..cee9c9e 100644
--- a/tests/test_aeap_transport.c
+++ b/tests/test_aeap_transport.c
@@ -134,22 +134,15 @@
 
 	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;
 }

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18719
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 19
Gerrit-Change-Id: I1711a331ecf5d35cd542911dc6aaa9acf1e172ad
Gerrit-Change-Number: 18719
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220628/5d5b7303/attachment.html>