[svn-commits] mjordan: branch 12 r417311 - in /branches/12: ./ channels/ channels/sip/inclu...
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Thu Jun 26 07:10:16 CDT 2014
Author: mjordan
Date: Thu Jun 26 07:10:11 2014
New Revision: 417311
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=417311
Log:
res_http_websocket: Close websocket correctly and use careful fwrite
When a client takes a long time to process information received from Asterisk,
a write operation using fwrite may fail to write all information. This causes
the underlying file stream to be in an unknown state, such that the socket
must be disconnected. Unfortunately, there are two problems with this in
Asterisk's existing websocket code:
1. Periodically, during the read loop, Asterisk must write to the connected
websocket to respond to pings. As such, Asterisk maintains a reference to
the session during the loop. When ast_http_websocket_write fails, it may
cause the session to decrement its ref count, but this in and of itself
does not break the read loop. The read loop's write, on the other hand,
does not break the loop if it fails. This causes the socket to get in a
'stuck' state, preventing the client from reconnecting to the server.
2. More importantly, however, is that the fwrite in ast_http_websocket_write
fails with a large volume of data when the client takes awhile to process
the information. When it does fail, it fails writing only a portion of
the bytes. With some debugging, it was shown that this was failing in a
similar fashion to ASTERISK-12767. Switching this over to ast_careful_fwrite
with a long enough timeout solved the problem.
Note that this version of the patch, unlike r417310 in Asterisk 11, exposes
configuration options beyond just chan_sip's sip.conf. Configuration options
to configure the write timeout have also been added to pjsip.conf and ari.conf.
#ASTERISK-23917 #close
Reported by: Matt Jordan
Review: https://reviewboard.asterisk.org/r/3624/
........
Merged revisions 417310 from http://svn.asterisk.org/svn/asterisk/branches/11
Modified:
branches/12/ (props changed)
branches/12/UPGRADE.txt
branches/12/channels/chan_sip.c
branches/12/channels/sip/include/sip.h
branches/12/configs/ari.conf.sample
branches/12/configs/pjsip.conf.sample
branches/12/configs/sip.conf.sample
branches/12/include/asterisk/http_websocket.h
branches/12/include/asterisk/res_pjsip.h
branches/12/res/ari/ari_websockets.c
branches/12/res/ari/config.c
branches/12/res/ari/internal.h
branches/12/res/res_ari.c
branches/12/res/res_http_websocket.c
branches/12/res/res_pjsip.c
branches/12/res/res_pjsip/config_transport.c
branches/12/res/res_pjsip_transport_websocket.c
Propchange: branches/12/
------------------------------------------------------------------------------
Binary property 'branch-11-merged' - no diff available.
Modified: branches/12/UPGRADE.txt
URL: http://svnview.digium.com/svn/asterisk/branches/12/UPGRADE.txt?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/UPGRADE.txt (original)
+++ branches/12/UPGRADE.txt Thu Jun 26 07:10:11 2014
@@ -34,6 +34,12 @@
accordingly. The changed strings are:
- "Exited on signal $EXITSIGNAL" => "Asterisk exited on signal $EXITSIGNAL."
- "Asterisk Died" => "Asterisk on $MACHINE died (sig $EXITSIGNAL)"
+
+ - Added a compatibility option for ari, chan_sip, and chan_pjsip
+ 'websocket_write_timeout'. When a websocket connection exists where Asterisk
+ writes a substantial amount of data to the connected client, and the connected
+ client is slow to process the received data, the socket may be disconnected.
+ In such cases, it may be necessary to adjust this value. Default is 100 ms.
From 12.3.0 to 12.3.1:
Modified: branches/12/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/channels/chan_sip.c?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/channels/chan_sip.c (original)
+++ branches/12/channels/chan_sip.c Thu Jun 26 07:10:11 2014
@@ -2668,6 +2668,10 @@
goto end;
}
+ if (ast_websocket_set_timeout(session, sip_cfg.websocket_write_timeout)) {
+ goto end;
+ }
+
while ((res = ast_wait_for_input(ast_websocket_fd(session), -1)) > 0) {
char *payload;
uint64_t payload_len;
@@ -32217,6 +32221,12 @@
ast_copy_string(default_parkinglot, v->value, sizeof(default_parkinglot));
} else if (!strcasecmp(v->name, "refer_addheaders")) {
global_refer_addheaders = ast_true(v->value);
+ } else if (!strcasecmp(v->name, "websocket_write_timeout")) {
+ if (sscanf(v->value, "%30d", &sip_cfg.websocket_write_timeout) != 1
+ || sip_cfg.websocket_write_timeout < 0) {
+ ast_log(LOG_WARNING, "'%s' is not a valid websocket_write_timeout value at line %d. Using default '%d'.\n", v->value, v->lineno, AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT);
+ sip_cfg.websocket_write_timeout = AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT;
+ }
}
}
Modified: branches/12/channels/sip/include/sip.h
URL: http://svnview.digium.com/svn/asterisk/branches/12/channels/sip/include/sip.h?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/channels/sip/include/sip.h (original)
+++ branches/12/channels/sip/include/sip.h Thu Jun 26 07:10:11 2014
@@ -771,6 +771,7 @@
struct ast_format_cap *caps; /*!< Supported codecs */
int tcp_enabled;
int default_max_forwards; /*!< Default max forwards (SIP Anti-loop) */
+ int websocket_write_timeout; /*!< Socket write timeout for websocket transports, in ms */
};
struct ast_websocket;
Modified: branches/12/configs/ari.conf.sample
URL: http://svnview.digium.com/svn/asterisk/branches/12/configs/ari.conf.sample?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/configs/ari.conf.sample (original)
+++ branches/12/configs/ari.conf.sample Thu Jun 26 07:10:11 2014
@@ -1,19 +1,25 @@
[general]
-enabled = yes ; When set to no, ARI support is disabled.
-;pretty = no ; When set to yes, responses from ARI are
-; ; formatted to be human readable.
-;allowed_origins = ; Comma separated list of allowed origins, for
-; ; Cross-Origin Resource Sharing. May be set to * to
-; ; allow all origins.
-;auth_realm = ; Realm to use for authentication. Defaults to Asterisk
-; ; REST Interface.
+enabled = yes ; When set to no, ARI support is disabled.
+;pretty = no ; When set to yes, responses from ARI are
+; ; formatted to be human readable.
+;allowed_origins = ; Comma separated list of allowed origins, for
+; ; Cross-Origin Resource Sharing. May be set to * to
+; ; allow all origins.
+;auth_realm = ; Realm to use for authentication. Defaults to Asterisk
+; ; REST Interface.
+;
+; 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.
+;websocket_write_timeout = 100
;[username]
-;type = user ; Specifies user configuration
-;read_only = no ; When set to yes, user is only authorized for
-; ; read-only requests.
+;type = user ; Specifies user configuration
+;read_only = no ; When set to yes, user is only authorized for
+; ; read-only requests.
;
-;password = ; Crypted or plaintext password (see password_format).
+;password = ; Crypted or plaintext password (see password_format).
;
; password_format may be set to plain (the default) or crypt. When set to crypt,
; crypt(3) is used to validate the password. A crypted password can be generated
@@ -22,3 +28,4 @@
; When set to plain, the password is in plaintext.
;
;password_format = plain
+
Modified: branches/12/configs/pjsip.conf.sample
URL: http://svnview.digium.com/svn/asterisk/branches/12/configs/pjsip.conf.sample?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/configs/pjsip.conf.sample (original)
+++ branches/12/configs/pjsip.conf.sample Thu Jun 26 07:10:11 2014
@@ -616,7 +616,13 @@
; "")
;tos=0 ; Enable TOS for the signalling sent over this transport (default: "0")
;cos=0 ; Enable COS for the signalling sent over this transport (default: "0")
-
+;websocket_write_timeout=100 ; Default write timeout to set on websocket
+ ; transports. 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.
;==========================CONTACT SECTION OPTIONS=========================
;[contact]
Modified: branches/12/configs/sip.conf.sample
URL: http://svnview.digium.com/svn/asterisk/branches/12/configs/sip.conf.sample?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/configs/sip.conf.sample (original)
+++ branches/12/configs/sip.conf.sample Thu Jun 26 07:10:11 2014
@@ -228,6 +228,12 @@
;tcpauthlimit = 100 ; tcpauthlimit specifies the maximum number of
; unauthenticated sessions that will be allowed
; to connect at any given time. (default: 100)
+
+;websocket_write_timeout = 100 ; Default write timeout to set on websocket transports.
+ ; 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.
transport=udp ; Set the default transports. The order determines the primary default transport.
; If tcpenable=no and the transport set is tcp, we will fallback to UDP.
Modified: branches/12/include/asterisk/http_websocket.h
URL: http://svnview.digium.com/svn/asterisk/branches/12/include/asterisk/http_websocket.h?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/include/asterisk/http_websocket.h (original)
+++ branches/12/include/asterisk/http_websocket.h Thu Jun 26 07:10:11 2014
@@ -23,6 +23,12 @@
#include "asterisk/optional_api.h"
#include <errno.h>
+
+/*! \brief Default websocket write timeout, in ms */
+#define AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT 100
+
+/*! \brief Default websocket write timeout, in ms (as a string) */
+#define AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT_STR "100"
/*!
* \file http_websocket.h
@@ -234,4 +240,15 @@
*/
AST_OPTIONAL_API(int, ast_websocket_set_nonblock, (struct ast_websocket *session), { errno = ENOSYS; return -1;});
+/*!
+ * \brief Set the timeout on a non-blocking WebSocket session.
+ *
+ * \since 11.11.0
+ * \since 12.4.0
+ *
+ * \retval 0 on success
+ * \retval -1 on failure
+ */
+AST_OPTIONAL_API(int, ast_websocket_set_timeout, (struct ast_websocket *session, int timeout), {return -1;});
+
#endif
Modified: branches/12/include/asterisk/res_pjsip.h
URL: http://svnview.digium.com/svn/asterisk/branches/12/include/asterisk/res_pjsip.h?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/include/asterisk/res_pjsip.h (original)
+++ branches/12/include/asterisk/res_pjsip.h Thu Jun 26 07:10:11 2014
@@ -126,6 +126,8 @@
unsigned int tos;
/*! QOS COS value */
unsigned int cos;
+ /*! Write timeout */
+ int write_timeout;
};
/*!
Modified: branches/12/res/ari/ari_websockets.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/ari/ari_websockets.c?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/res/ari/ari_websockets.c (original)
+++ branches/12/res/ari/ari_websockets.c Thu Jun 26 07:10:11 2014
@@ -56,8 +56,13 @@
struct ast_websocket *ws_session, int (*validator)(struct ast_json *))
{
RAII_VAR(struct ast_ari_websocket_session *, session, NULL, ao2_cleanup);
+ RAII_VAR(struct ast_ari_conf *, config, ast_ari_config_get(), ao2_cleanup);
if (ws_session == NULL) {
+ return NULL;
+ }
+
+ if (config == NULL || config->general == NULL) {
return NULL;
}
@@ -70,6 +75,11 @@
"ARI web socket failed to set nonblock; closing: %s\n",
strerror(errno));
return NULL;
+ }
+
+ if (ast_websocket_set_timeout(ws_session, config->general->write_timeout)) {
+ ast_log(LOG_WARNING, "Failed to set write timeout %d on ARI web socket\n",
+ config->general->write_timeout);
}
session = ao2_alloc(sizeof(*session), websocket_session_dtor);
Modified: branches/12/res/ari/config.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/ari/config.c?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/res/ari/config.c (original)
+++ branches/12/res/ari/config.c Thu Jun 26 07:10:11 2014
@@ -27,6 +27,7 @@
ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
#include "asterisk/config_options.h"
+#include "asterisk/http_websocket.h"
#include "internal.h"
/*! \brief Locking container for safe configuration access. */
@@ -320,6 +321,9 @@
aco_option_register(&cfg_info, "allowed_origins", ACO_EXACT, general_options,
"", OPT_STRINGFIELD_T, 0,
STRFLDSET(struct ast_ari_conf_general, allowed_origins));
+ aco_option_register(&cfg_info, "websocket_write_timeout", ACO_EXACT, general_options,
+ AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT_STR, OPT_INT_T, PARSE_IN_RANGE,
+ FLDSET(struct ast_ari_conf_general, write_timeout), 1, INT_MAX);
aco_option_register(&cfg_info, "type", ACO_EXACT, user, NULL,
OPT_NOOP_T, 0, 0);
Modified: branches/12/res/ari/internal.h
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/ari/internal.h?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/res/ari/internal.h (original)
+++ branches/12/res/ari/internal.h Thu Jun 26 07:10:11 2014
@@ -65,6 +65,8 @@
struct ast_ari_conf_general {
/*! Enabled by default, disabled if false. */
int enabled;
+ /*! Write timeout for websocket connections */
+ int write_timeout;
/*! Encoding format used during output (default compact). */
enum ast_json_encoding_format format;
/*! Authentication realm */
Modified: branches/12/res/res_ari.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_ari.c?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/res/res_ari.c (original)
+++ branches/12/res/res_ari.c Thu Jun 26 07:10:11 2014
@@ -94,6 +94,14 @@
<ref type="filename">http.conf</ref>
<ref type="link">https://wiki.asterisk.org/wiki/display/AST/Asterisk+Builtin+mini-HTTP+Server</ref>
</see-also>
+ </configOption>
+ <configOption name="websocket_write_timeout">
+ <synopsis>The timeout (in milliseconds) to set on WebSocket connections.</synopsis>
+ <description>
+ <para>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; default is 100 ms.</para>
+ </description>
</configOption>
<configOption name="pretty">
<synopsis>Responses from ARI are formatted to be human readable</synopsis>
Modified: branches/12/res/res_http_websocket.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_http_websocket.c?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/res/res_http_websocket.c (original)
+++ branches/12/res/res_http_websocket.c Thu Jun 26 07:10:11 2014
@@ -77,6 +77,7 @@
size_t payload_len; /*!< Length of the payload */
char *payload; /*!< Pointer to the payload */
size_t reconstruct; /*!< Number of bytes before a reconstructed payload will be returned and a new one started */
+ int timeout; /*!< The timeout for operations on the socket */
unsigned int secure:1; /*!< Bit to indicate that the transport is secure */
unsigned int closing:1; /*!< Bit to indicate that the session is in the process of being closed */
unsigned int close_sent:1; /*!< Bit to indicate that the session close opcode has been sent and no further data will be sent */
@@ -255,7 +256,7 @@
session->close_sent = 1;
ao2_lock(session);
- res = (fwrite(frame, 1, 4, session->f) == 4) ? 0 : -1;
+ res = ast_careful_fwrite(session->f, session->fd, frame, 4, session->timeout);
ao2_unlock(session);
return res;
}
@@ -298,13 +299,12 @@
ao2_unlock(session);
return -1;
}
-
- if (fwrite(frame, 1, header_size, session->f) != header_size) {
+ if (ast_careful_fwrite(session->f, session->fd, frame, header_size, session->timeout)) {
ao2_unlock(session);
return -1;
}
- if (fwrite(payload, 1, actual_length, session->f) != actual_length) {
+ if (ast_careful_fwrite(session->f, session->fd, payload, actual_length, session->timeout)) {
ao2_unlock(session);
return -1;
}
@@ -362,6 +362,13 @@
if ((flags = fcntl(session->fd, F_SETFL, flags)) == -1) {
return -1;
}
+
+ return 0;
+}
+
+int AST_OPTIONAL_API_NAME(ast_websocket_set_timeout)(struct ast_websocket *session, int timeout)
+{
+ session->timeout = timeout;
return 0;
}
@@ -510,8 +517,10 @@
}
/* Per the RFC for PING we need to send back an opcode with the application data as received */
- if (*opcode == AST_WEBSOCKET_OPCODE_PING) {
- ast_websocket_write(session, AST_WEBSOCKET_OPCODE_PONG, *payload, *payload_len);
+ if ((*opcode == AST_WEBSOCKET_OPCODE_PING) && (ast_websocket_write(session, AST_WEBSOCKET_OPCODE_PONG, *payload, *payload_len))) {
+ *payload_len = 0;
+ ast_websocket_close(session, 1009);
+ return 0;
}
session->payload = new_payload;
@@ -685,6 +694,7 @@
ao2_ref(protocol_handler, -1);
return 0;
}
+ session->timeout = AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT;
combined = ast_alloca(combined_length);
snprintf(combined, combined_length, "%s%s", key, WEBSOCKET_GUID);
Modified: branches/12/res/res_pjsip.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_pjsip.c?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/res/res_pjsip.c (original)
+++ branches/12/res/res_pjsip.c Thu Jun 26 07:10:11 2014
@@ -867,6 +867,14 @@
for more information on this parameter.</para>
<note><para>This option does not apply to the <replaceable>ws</replaceable>
or the <replaceable>wss</replaceable> protocols.</para></note>
+ </description>
+ </configOption>
+ <configOption name="websocket_write_timeout">
+ <synopsis>The timeout (in milliseconds) to set on WebSocket connections.</synopsis>
+ <description>
+ <para>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; default is 100 ms.</para>
</description>
</configOption>
</configObject>
Modified: branches/12/res/res_pjsip/config_transport.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_pjsip/config_transport.c?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/res/res_pjsip/config_transport.c (original)
+++ branches/12/res/res_pjsip/config_transport.c Thu Jun 26 07:10:11 2014
@@ -28,6 +28,7 @@
#include "asterisk/sorcery.h"
#include "asterisk/acl.h"
#include "include/res_pjsip_private.h"
+#include "asterisk/http_websocket.h"
static int sip_transport_to_ami(const struct ast_sip_transport *transport,
struct ast_str **buf)
@@ -668,6 +669,7 @@
ast_sorcery_object_field_register_custom(sorcery, "transport", "local_net", "", transport_localnet_handler, localnet_to_str, localnet_to_vl, 0, 0);
ast_sorcery_object_field_register_custom(sorcery, "transport", "tos", "0", transport_tos_handler, tos_to_str, NULL, 0, 0);
ast_sorcery_object_field_register(sorcery, "transport", "cos", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_transport, cos));
+ ast_sorcery_object_field_register(sorcery, "transport", "websocket_write_timeout", AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT_STR, OPT_INT_T, PARSE_IN_RANGE, FLDSET(struct ast_sip_transport, write_timeout), 1, INT_MAX);
ast_sip_register_endpoint_formatter(&endpoint_transport_formatter);
Modified: branches/12/res/res_pjsip_transport_websocket.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_pjsip_transport_websocket.c?view=diff&rev=417311&r1=417310&r2=417311
==============================================================================
--- branches/12/res/res_pjsip_transport_websocket.c (original)
+++ branches/12/res/res_pjsip_transport_websocket.c Thu Jun 26 07:10:11 2014
@@ -207,6 +207,37 @@
return (read_data->payload_len == recvd) ? 0 : -1;
}
+static int get_write_timeout(void)
+{
+ int write_timeout = -1;
+ struct ao2_container *transports;
+
+ transports = ast_sorcery_retrieve_by_fields(ast_sip_get_sorcery(), "transport", AST_RETRIEVE_FLAG_ALL, NULL);
+
+ if (transports) {
+ struct ao2_iterator it_transports = ao2_iterator_init(transports, 0);
+ struct ast_sip_transport *transport;
+
+ for (; (transport = ao2_iterator_next(&it_transports)); ao2_cleanup(transport)) {
+ if (transport->type != AST_TRANSPORT_WS && transport->type != AST_TRANSPORT_WSS) {
+ continue;
+ }
+ ast_debug(5, "Found %s transport with write timeout: %d\n",
+ transport->type == AST_TRANSPORT_WS ? "WS" : "WSS",
+ transport->write_timeout);
+ write_timeout = MAX(write_timeout, transport->write_timeout);
+ }
+ ao2_cleanup(transports);
+ }
+
+ if (write_timeout < 0) {
+ write_timeout = AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT;
+ }
+
+ ast_debug(1, "Write timeout for WS/WSS transports: %d\n", write_timeout);
+ return write_timeout;
+}
+
/*!
\brief WebSocket connection handler.
*/
@@ -218,6 +249,11 @@
struct transport_read_data read_data;
if (ast_websocket_set_nonblock(session)) {
+ ast_websocket_unref(session);
+ return;
+ }
+
+ if (ast_websocket_set_timeout(session, get_write_timeout())) {
ast_websocket_unref(session);
return;
}
More information about the svn-commits
mailing list