diff --git a/channels/chan_websocket.c b/channels/chan_websocket.c index 724c9263fb..2667a81c10 100644 --- a/channels/chan_websocket.c +++ b/channels/chan_websocket.c @@ -68,6 +68,7 @@ static const char *msg_format_map[] = { struct webchan_conf_global { SORCERY_OBJECT(details); enum webchan_control_msg_format control_msg_format; + int write_timeout; }; /* This is from the perspective of the app, NOT Asterisk */ @@ -534,10 +535,15 @@ static struct ast_frame *dequeue_frame(struct websocket_pvt *instance) * We just need to send the data to the websocket. * The data should already be NULL terminated. */ - ast_websocket_write_string(instance->websocket, + int res = ast_websocket_write_string(instance->websocket, queued_frame->data.ptr); - ast_debug(4, "%s: Sent %s\n", - ast_channel_name(instance->channel), (char *)queued_frame->data.ptr); + if (res != 0) { + ast_log(LOG_ERROR, "%s: Unable to send event %s\n", + ast_channel_name(instance->channel), (char *)queued_frame->data.ptr); + } else { + ast_debug(4, "%s: Sent %s\n", + ast_channel_name(instance->channel), (char *)queued_frame->data.ptr); + } } /* * We do NOT send these to the core so we need to free @@ -1213,6 +1219,14 @@ static int websocket_handoff_to_channel(struct websocket_pvt *instance) ast_log(LOG_WARNING, "Failed to set TCP_NODELAY on websocket connection: %s\n", strerror(errno)); } + /* + * The way write timeouts are handled in iostream requires the socket to be + * in non-blocking mode. This is fine for reads as well because we already + * set the websocket file descriptor on the channel and let it call + * webchan_read() when data is available. + */ + ast_websocket_set_nonblock(instance->websocket); + /* * Tell res_http_websocket to accumulate incoming WebSocket CONTINUATION frames * into chunks of 1024 bytes and send us a TEXT or BINARY frame when the threshold @@ -1266,6 +1280,7 @@ static void _websocket_request_hangup(struct websocket_pvt *instance, int ast_ca static int webchan_write(struct ast_channel *ast, struct ast_frame *f) { struct websocket_pvt *instance = ast_channel_tech_pvt(ast); + int res = 0; if (!instance || !instance->websocket) { ast_log(LOG_WARNING, "%s: WebSocket instance or client not found\n", @@ -1295,8 +1310,13 @@ static int webchan_write(struct ast_channel *ast, struct ast_frame *f) return -1; } - return ast_websocket_write(instance->websocket, AST_WEBSOCKET_OPCODE_BINARY, + res = ast_websocket_write(instance->websocket, AST_WEBSOCKET_OPCODE_BINARY, (char *)f->data.ptr, (uint64_t)f->datalen); + if (res != 0) { + ast_log(LOG_WARNING, "%s: WebSocket write failure\n", ast_channel_name(ast)); + } + + return res; } /*! @@ -1311,6 +1331,10 @@ static int webchan_call(struct ast_channel *ast, const char *dest, { struct websocket_pvt *instance = ast_channel_tech_pvt(ast); enum ast_websocket_result result; + struct webchan_conf_global *global_cfg = ast_sorcery_retrieve_by_id(sorcery, "global", "global"); + int global_write_timeout = global_cfg ? global_cfg->write_timeout : AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT; + + ao2_cleanup(global_cfg); if (!instance) { ast_log(LOG_WARNING, "%s: WebSocket instance not found\n", @@ -1348,6 +1372,21 @@ static int webchan_call(struct ast_channel *ast, const char *dest, return -1; } + /* + * If websocket_client->write_timeout was set in websocket_client.conf, it will + * have been applied to the websocket by ast_websocket_client_connect() above. + * If it wasn't set in websocket_client.conf, the value will be INT_MAX and + * and ast_websocket_client_connect() will have set AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT + * on the websocket. However, the user may have set write_timeout in the global section + * of chan_websocket.conf so if it wasn't set in websocket_client.conf, we'll now set + * the websocket timeout to that. If they haven't set it in chan_websocket.conf either, + * it'll default to AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT as well so the call below will + * basically become a no-op. + */ + if (instance->client->write_timeout == INT_MAX) { + ast_websocket_set_timeout(instance->websocket, global_write_timeout); + } + return websocket_handoff_to_channel(instance); } @@ -1877,6 +1916,10 @@ static void incoming_ws_established_cb(struct ast_websocket *ast_ws_session, struct ast_variable *v; const char *connection_id = NULL; struct websocket_pvt *instance = NULL; + struct webchan_conf_global *global_cfg = ast_sorcery_retrieve_by_id(sorcery, "global", "global"); + int global_write_timeout = global_cfg ? global_cfg->write_timeout : AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT; + + ao2_cleanup(global_cfg); ast_debug(3, "WebSocket established\n"); @@ -1915,6 +1958,8 @@ static void incoming_ws_established_cb(struct ast_websocket *ast_ws_session, } instance->websocket = ao2_bump(ast_ws_session); + ast_websocket_set_timeout(instance->websocket, global_write_timeout); + websocket_handoff_to_channel(instance); ao2_cleanup(instance); /* @@ -2066,6 +2111,11 @@ static int global_apply(const struct ast_sorcery *sorcery, void *obj) ast_debug(1, "control_msg_format: %s\n", control_msg_format_to_str(cfg->control_msg_format)); + if (cfg->write_timeout <= 0) { + ast_log(LOG_WARNING, "The write_timeout parameter must be > 0\n"); + return -1; + } + return 0; } @@ -2090,6 +2140,8 @@ static int load_config(void) ast_sorcery_object_field_register_nodoc(sorcery, "global", "type", "", OPT_NOOP_T, 0, 0); ast_sorcery_register_cust(global, control_message_format, "plain-text"); + ast_sorcery_register_int(global, webchan_conf_global, write_timeout, write_timeout, + AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT); ast_sorcery_load(sorcery); diff --git a/channels/chan_websocket_doc.xml b/channels/chan_websocket_doc.xml index ca836a026e..a0714feff5 100644 --- a/channels/chan_websocket_doc.xml +++ b/channels/chan_websocket_doc.xml @@ -35,6 +35,34 @@ + + + 20.22.0 + 22.12.0 + 23.6.0 + 24.0.0 + + Write timeout (ms). + + + The maximum number of milliseconds to wait for a write + to the WebSocket to succeed. The WebSocket will be closed and the + channel hung up when the timeout is reached. For outgoing + WebSocket connections, this sets the default timeout which can be + overridden in websocket_client.conf. + + + + Success means the write request was accepted by the + operating system and does not imply the payload was + actually transmitted or received by the server. + + + + + write_timeout + + diff --git a/configs/samples/ari.conf.sample b/configs/samples/ari.conf.sample index 04973e10b4..6fc3f40e29 100644 --- a/configs/samples/ari.conf.sample +++ b/configs/samples/ari.conf.sample @@ -10,8 +10,9 @@ enabled = yes ; When set to no, ARI support is disabled. ; ; Default write timeout to set on websockets. This value may need to be adjusted ; for connections where Asterisk must write a substantial amount of data and the -; receiving clients are slow to process the received information. Value is in -; milliseconds; default is 100 ms. +; receiving clients are slow to process the received information. Outgoing WebSocket +; connections can override this in websocket_client.conf. +; Value is in milliseconds; default is 100 ms. ;websocket_write_timeout = 100 ; ; Display certain channel variables every time a channel-oriented diff --git a/configs/samples/chan_websocket.conf.sample b/configs/samples/chan_websocket.conf.sample index bae06f9541..5aacb5b755 100644 --- a/configs/samples/chan_websocket.conf.sample +++ b/configs/samples/chan_websocket.conf.sample @@ -8,3 +8,15 @@ ; json: All messages are properly formatted ; JSON. ; Default: plain-text + +;write_timeout = 100 ; Write timeout in milliseconds. + ; The maximum number of milliseconds to wait for a write + ; to the WebSocket to succeed. The WebSocket will be closed + ; and the channel hung up when the timeout is reached. + ; Outgoing WebSocket connections can override this in + ; websocket_client.conf. + ; NOTE: Success means the write request was accepted by the + ; operating system and does not imply the payload was + ; actually transmitted or received by the server. + ; Default: 100 + \ No newline at end of file diff --git a/configs/samples/websocket_client.conf.sample b/configs/samples/websocket_client.conf.sample index 706646becf..7df1d745cd 100644 --- a/configs/samples/websocket_client.conf.sample +++ b/configs/samples/websocket_client.conf.sample @@ -14,6 +14,10 @@ ;password = password ; The authentication password for the username. ; Default: none ;connection_timeout = 500 ; Connection timeout in milliseconds. + ; The maximum number of milliseconds to wait for a connection + ; to the WebSocket server to succeed. If the timer expires, + ; reconnection will be attempted based on the settings of the + ; reconnect_attempts and reconnect_interval parameters. ; Default: 500 ;reconnect_interval = 1000 ; Number of milliseconds between (re)connection ; attempts. @@ -24,6 +28,15 @@ ; always retry forever but this setting will control ; how often failure messages are logged. ; Default: 4 for both connection types. +;write_timeout = 100 ; Write timeout in milliseconds. + ; The maximum number of milliseconds to wait for a write + ; to the WebSocket to succeed. If this connection is used + ; by chan_websocket, the WebSocket will be closed and the + ; channel hung up when the timeout is reached. + ; NOTE: Success means the write request was accepted by the + ; operating system and does not imply the payload was + ; actually transmitted or received by the server. + ; Default: 100 ;tls_enabled = no ; Set to "yes" to enable TLS connections. ; Default: no ;ca_list_file = /etc/pki/tls/cert.pem diff --git a/include/asterisk/http_websocket.h b/include/asterisk/http_websocket.h index 8e542b8a9a..54e0910132 100644 --- a/include/asterisk/http_websocket.h +++ b/include/asterisk/http_websocket.h @@ -548,6 +548,14 @@ struct ast_websocket_client_options { int pingpongs; /*!< Enable Websocket PING/PONGs */ unsigned int pingpong_interval; /*!< Send PING messages at this interval in seconds */ unsigned int pingpong_probes; /*!< Close connection after this many missed responses */ + /*! + * Optional write timeout + * + * How long (in milliseconds) to wait for a write to the websocket to complete. + * \warning This parameter is ignored if the WebSocket is in blocking mode. + * Ensure ast_websocket_set_nonblock() is called before calling ast_websocket_write(). + */ + int write_timeout; }; /*! @@ -589,6 +597,11 @@ AST_OPTIONAL_API(const char *, ast_websocket_client_accept_protocol, * \since 11.11.0 * \since 12.4.0 * + * \warning To be effective, the socket must be in non-blocking mode because the timeout + * can only be checked after a read or write operation returns. If the socket is in blocking + * mode (the default), those calls may block for longer than the specified timeout, possibly + * much longer. + * * \retval 0 on success * \retval -1 on failure */ diff --git a/include/asterisk/iostream.h b/include/asterisk/iostream.h index 9d88e3922f..a0570559a4 100644 --- a/include/asterisk/iostream.h +++ b/include/asterisk/iostream.h @@ -89,6 +89,11 @@ void ast_iostream_set_timeout_idle_inactivity(struct ast_iostream *stream, int t * to complete an operation that can take several I/O calls. The * main use is as an authentication timer with us. * + * \warning To be effective, the socket must be in non-blocking mode because the timeout + * can only be checked after a read or write operation returns. If the socket is in blocking + * mode (the default), those calls may block for longer than the specified timeout, possibly + * much longer. + * * \note Setting timeout to -1 disables the timeout. * \note Setting this timeout replaces the inactivity timeout timer. */ diff --git a/include/asterisk/websocket_client.h b/include/asterisk/websocket_client.h index 0b12e62ba6..c88449ce51 100644 --- a/include/asterisk/websocket_client.h +++ b/include/asterisk/websocket_client.h @@ -52,6 +52,7 @@ enum ast_ws_client_fields { AST_WS_CLIENT_FIELD_PINGPONGS = (1ULL << 26), AST_WS_CLIENT_FIELD_PINGPONG_INTERVAL = (1ULL << 27), AST_WS_CLIENT_FIELD_PINGPONG_PROBES = (1ULL << 28), + AST_WS_CLIENT_FIELD_WRITE_TIMEOUT = (1ULL << 29), AST_WS_CLIENT_NEEDS_RECONNECT = AST_WS_CLIENT_FIELD_URI | AST_WS_CLIENT_FIELD_PROTOCOLS | AST_WS_CLIENT_FIELD_CONNECTION_TYPE | AST_WS_CLIENT_FIELD_USERNAME | AST_WS_CLIENT_FIELD_PASSWORD @@ -65,7 +66,7 @@ enum ast_ws_client_fields { | AST_WS_CLIENT_FIELD_TCP_KEEPALIVE_TIME | AST_WS_CLIENT_FIELD_TCP_KEEPALIVE_INTERVAL | AST_WS_CLIENT_FIELD_TCP_KEEPALIVE_PROBES | AST_WS_CLIENT_FIELD_PINGPONGS | AST_WS_CLIENT_FIELD_PINGPONG_INTERVAL - | AST_WS_CLIENT_FIELD_PINGPONG_PROBES, + | AST_WS_CLIENT_FIELD_PINGPONG_PROBES | AST_WS_CLIENT_FIELD_WRITE_TIMEOUT, }; /* @@ -105,6 +106,7 @@ struct ast_websocket_client { int pingpongs; /*!< Enable WebSocket PING/PONGs */ unsigned int pingpong_interval; /*!< Send WebSocket PINGs at this interval in seconds */ unsigned int pingpong_probes; /*!< Close connection after this many missed PONG responses */ + int write_timeout; /*!< Write timeout (ms) */ }; /*! diff --git a/res/ari/ari_doc.xml b/res/ari/ari_doc.xml index dd7e54cdfa..07544c4653 100644 --- a/res/ari/ari_doc.xml +++ b/res/ari/ari_doc.xml @@ -30,12 +30,25 @@ 11.11.0 12.4.0 - The timeout (in milliseconds) to set on WebSocket connections. + Write timeout (ms). - If a websocket connection accepts input slowly, the timeout - for writes to it can be increased to keep it from being disconnected. - Value is in milliseconds. + + The maximum number of milliseconds to wait for a write + to the WebSocket to succeed. The WebSocket will be closed + when the timeout is reached. For outgoing WebSocket connections, + this sets the default timeout which can be overridden in websocket_client.conf. + + + + Success means the write request was accepted by the + operating system and does not imply the payload was + actually transmitted or received by the server. + + + + write_timeout + diff --git a/res/ari/ari_websockets.c b/res/ari/ari_websockets.c index 13d998317a..ce21b1e8fa 100644 --- a/res/ari/ari_websockets.c +++ b/res/ari/ari_websockets.c @@ -703,17 +703,12 @@ static struct ari_ws_session *session_create( static int session_update(struct ari_ws_session *ari_ws_session, struct ast_websocket *ast_ws_session, int send_registered_events) { - RAII_VAR(struct ari_conf_general *, general, ari_conf_get_general(), ao2_cleanup); int i; if (ast_ws_session == NULL) { return -1; } - if (!general) { - return -1; - } - ari_ws_session->remote_addr = ast_strdup(ast_sockaddr_stringify( ast_websocket_remote_address(ast_ws_session))); if (!ari_ws_session->remote_addr) { @@ -728,11 +723,6 @@ static int session_update(struct ari_ws_session *ari_ws_session, return -1; } - if (ast_websocket_set_timeout(ast_ws_session, general->write_timeout)) { - ast_log(LOG_WARNING, "Failed to set write timeout %d on ARI web socket\n", - general->write_timeout); - } - ast_websocket_ref(ast_ws_session); ari_ws_session->ast_ws_session = ast_ws_session; ao2_lock(ari_ws_session); @@ -818,9 +808,12 @@ static void websocket_established_cb(struct ast_websocket *ast_ws_session, char *remote_addr = ast_sockaddr_stringify( ast_websocket_remote_address(ast_ws_session)); const char *session_id = ast_websocket_session_id(ast_ws_session); - + struct ari_conf_general *general = ari_conf_get_general(); + int general_write_timeout = general ? general->write_timeout : AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT; SCOPE_ENTER(2, "%s: WebSocket established\n", remote_addr); + ao2_cleanup(general); + if (TRACE_ATLEAST(2)) { ast_trace(2, "%s: Websocket Upgrade Headers:\n", remote_addr); for (v = upgrade_headers; v; v = v->next) { @@ -844,6 +837,8 @@ static void websocket_established_cb(struct ast_websocket *ast_ws_session, remote_addr, session_id); } + ast_websocket_set_timeout(ast_ws_session, general_write_timeout); + /* * Since this is a new inbound websocket session, * session_register_apps() will have already sent "ApplicationRegistered" @@ -951,7 +946,10 @@ static void *outbound_session_handler_thread(void *obj) { struct ari_ws_session *session = obj; int already_sent_registers = 1; + struct ari_conf_general *general = ari_conf_get_general(); + int general_write_timeout = general ? general->write_timeout : AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT; + ao2_cleanup(general); /* * We use pthread_cleanup_push because RAII destructors don't run * if we cancel the thread. @@ -969,6 +967,7 @@ static void *outbound_session_handler_thread(void *obj) enum ast_websocket_result result; struct ast_json *msg; + ast_debug(3, "%s: Attempting to connect to %s\n", session->session_id, session->owc->websocket_client->uri); @@ -1005,6 +1004,21 @@ static void *outbound_session_handler_thread(void *obj) session->type == AST_WS_TYPE_CLIENT_PERSISTENT ? session->session_id : session->channel_name, session->owc->websocket_client->uri); + /* + * If websocket_client->write_timeout was set in websocket_client.conf, it will + * have been applied to the websocket by ast_websocket_client_connect() above. + * If it wasn't set in websocket_client.conf, the value will be INT_MAX and + * and ast_websocket_client_connect() will have set AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT + * on the websocket. However, the user may have set write_timeout in the general section + * of ari.conf so if it wasn't set in websocket_client.conf, we'll now set the websocket + * timeout to that. If they haven't set it in ari.conf either, it'll default to + * AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT as well so the call below will basically + * become a no-op. + */ + if (session->owc->websocket_client->write_timeout == INT_MAX) { + ast_websocket_set_timeout(astws, general_write_timeout); + } + /* * We only want to send "ApplicationRegistered" events in the * case of a reconnect. The initial connection will have already sent diff --git a/res/ari/config.c b/res/ari/config.c index 6575ef7a66..2223da003e 100644 --- a/res/ari/config.c +++ b/res/ari/config.c @@ -461,6 +461,11 @@ static int general_apply(const struct ast_sorcery *sorcery, void *obj) ast_debug(2, "Initializing general config\n"); + if (general->write_timeout <= 0) { + ast_log(LOG_WARNING, "The websocket_write_timeout parameter must be > 0\n"); + return -1; + } + parse = ast_strdupa(general->channelvars); AST_STANDARD_APP_ARGS(args, parse); @@ -742,7 +747,6 @@ static int ari_conf_init(void) ast_sorcery_register_int(general, ari_conf_general, websocket_write_timeout, write_timeout, AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT); - ast_sorcery_object_field_register(sorcery, "user", "type", "", OPT_NOOP_T, 0, 0); ast_sorcery_register_sf(user, ari_conf_user, password, password, ""); ast_sorcery_register_bool(user, ari_conf_user, read_only, read_only, "no"); diff --git a/res/res_http_websocket.c b/res/res_http_websocket.c index 47e793bce3..9d53ec7acf 100644 --- a/res/res_http_websocket.c +++ b/res/res_http_websocket.c @@ -1555,7 +1555,7 @@ static struct ast_websocket * websocket_client_create( ws->client->version = 13; ws->opcode = -1; ws->reconstruct = DEFAULT_RECONSTRUCTION_CEILING; - ws->timeout = options->timeout; + ws->timeout = options->write_timeout; return ws; } @@ -1920,6 +1920,7 @@ struct ast_websocket *AST_OPTIONAL_API_NAME(ast_websocket_client_create) .protocols = protocols, .timeout = -1, .tls_cfg = tls_cfg, + .write_timeout = AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT, }; return ast_websocket_client_create_with_options(&options, result); diff --git a/res/res_websocket_client.c b/res/res_websocket_client.c index eff603ee58..9b74f33ec4 100644 --- a/res/res_websocket_client.c +++ b/res/res_websocket_client.c @@ -139,6 +139,20 @@ verify_server_hostname = no 22.5.0 Connection timeout (ms). + + + The maximum number of milliseconds to wait for a connection + to the WebSocket server to succeed. If the timer expires, + reconnection will be attempted based on the settings of the + reconnect_attempts and reconnect_interval + parameters. + + + + write_timeout + reconnect_attempts + reconnect_interval + @@ -156,6 +170,10 @@ verify_server_hostname = no how often failure messages are logged. + + connection_timeout + reconnect_interval + @@ -164,6 +182,38 @@ verify_server_hostname = no 22.5.0 How often should reconnection be attempted (ms)? + + connection_timeout + reconnect_attempts + + + + + 20.22.0 + 22.12.0 + 23.6.0 + 24.0.0 + + Write timeout (ms). + + + The maximum number of milliseconds to wait for a write + to the WebSocket to succeed. If this connection is used + by chan_websocket, the WebSocket will be closed and the + channel hung up when the timeout is reached. The default + value is set by the module creating the client. + + + + Success means the write request was accepted by the + operating system and does not imply the payload was + actually transmitted or received by the server. + + + + + connection_timeout + @@ -397,6 +447,7 @@ struct ast_websocket *ast_websocket_client_connect(struct ast_websocket_client * .username = wc->username, .password = wc->password, .timeout = wc->connect_timeout, + .write_timeout = wc->write_timeout != INT_MAX ? wc->write_timeout : AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT, .suppress_connection_msgs = 1, .proxy_host = wc->proxy_host, .proxy_username = wc->proxy_username, @@ -612,6 +663,12 @@ static int wc_apply(const struct ast_sorcery *sorcery, void *obj) res = -1; } } + + if (wc->write_timeout <= 0) { + ast_log(LOG_WARNING, "The write_timeout parameter must be > 0\n"); + res = -1; + } + if (res != 0) { ast_log(LOG_WARNING, "%s: Websocket client configuration failed\n", id); } else { @@ -720,6 +777,8 @@ enum ast_ws_client_fields ast_websocket_client_get_field_diff( changed |= AST_WS_CLIENT_FIELD_PINGPONG_INTERVAL; } else if (ast_strings_equal(v->name, "pingpong_probes")) { changed |= AST_WS_CLIENT_FIELD_PINGPONG_PROBES; + } else if (ast_strings_equal(v->name, "write_timeout")) { + changed |= AST_WS_CLIENT_FIELD_WRITE_TIMEOUT; } else { ast_debug(2, "%s: Unknown change %s\n", new_id, v->name); } @@ -803,6 +862,11 @@ static int load_module(void) ast_sorcery_register_bool(websocket_client, ast_websocket_client, enable_pingpongs, pingpongs, "no"); ast_sorcery_register_uint(websocket_client, ast_websocket_client, pingpong_interval, pingpong_interval, 20); ast_sorcery_register_uint(websocket_client, ast_websocket_client, pingpong_probes, pingpong_probes, 3); + /* + * The default for write_timeout needs to be INT_MAX so we can tell that it's not set. + * This allows the modules using this capability to apply their own default value. + */ + ast_sorcery_register_int(websocket_client, ast_websocket_client, write_timeout, write_timeout, INT_MAX); ast_sorcery_load(sorcery);