<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3659/">https://reviewboard.asterisk.org/r/3659/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/3659/diff/2/?file=60725#file60725line521" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/res/res_http_websocket.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int AST_OPTIONAL_API_NAME(ast_websocket_read)(struct ast_websocket *session, char **payload, uint64_t *payload_len, enum ast_websocket_opcode *opcode, int *fragmented)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">513</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="o">*</span><span class="n">opcode</span> <span class="o">==</span> <span class="n">AST_WEBSOCKET_OPCODE_PING</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">520</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="o">*</span><span class="n">opcode</span> <span class="o">==</span> <span class="n">AST_WEBSOCKET_OPCODE_PING</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">514</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="n">ast_websocket_write</span><span class="p">(</span><span class="n">session</span><span class="p">,</span> <span class="n">AST_WEBSOCKET_OPCODE_PONG</span><span class="p">,</span> <span class="o">*</span><span class="n">payload</span><span class="p">,</span> <span class="o">*</span><span class="n">payload_len</span><span class="p">)<span class="hl">;</span></span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">521</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="k"><span class="hl">if</span></span><span class="hl"> </span><span class="p"><span class="hl">(</span></span><span class="n">ast_websocket_write</span><span class="p">(</span><span class="n">session</span><span class="p">,</span> <span class="n">AST_WEBSOCKET_OPCODE_PONG</span><span class="p">,</span> <span class="o">*</span><span class="n">payload</span><span class="p">,</span> <span class="o">*</span><span class="n">payload_len</span><span class="p">)<span class="hl">)</span></span><span class="hl"> </span><span class="p"><span class="hl">{</span></span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">To save some indentation, you could combine these two if statements into one compound one:
if (*opcode == AST_WEBSOCKET_PING && ast_websocket_write(session, AST_WEBSOCKET_OPCODE_PONG, *payload, *payload_len)) {
...
}</pre>
</div>
<br />
<p>- Mark Michelson</p>
<br />
<p>On June 25th, 2014, 6:38 p.m. UTC, Matt Jordan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated June 25, 2014, 6:38 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-23917">ASTERISK-23917</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">*Note:* This issue was originally seen when sending large volumes of data to a connected ARI client over a websocket, but it theoretically could occur with any version of Asterisk that uses a websocket. This patch is for Asterisk 12. The patch for Asterisk 11 only is on https://reviewboard.asterisk.org/r/3624.
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.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Prior to the patch (using Asterisk 12), sending information to a connected ARI client for a large number of channels would periodically cause a disconnect. Once disconnected, the client could not re-connect.
With the patch, the disconnects stopped. By setting the write timeout to a very low value, the disconnects occurred again, and the client was seen to reconnect (as the previous socket was completely closed).</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/12/res/res_pjsip_transport_websocket.c <span style="color: grey">(417210)</span></li>
<li>/branches/12/res/res_pjsip/config_transport.c <span style="color: grey">(417210)</span></li>
<li>/branches/12/res/res_pjsip.c <span style="color: grey">(417210)</span></li>
<li>/branches/12/res/res_http_websocket.c <span style="color: grey">(417210)</span></li>
<li>/branches/12/res/res_ari.c <span style="color: grey">(417210)</span></li>
<li>/branches/12/res/ari/internal.h <span style="color: grey">(417210)</span></li>
<li>/branches/12/res/ari/config.c <span style="color: grey">(417210)</span></li>
<li>/branches/12/res/ari/ari_websockets.c <span style="color: grey">(417210)</span></li>
<li>/branches/12/include/asterisk/res_pjsip.h <span style="color: grey">(417210)</span></li>
<li>/branches/12/include/asterisk/http_websocket.h <span style="color: grey">(417210)</span></li>
<li>/branches/12/configs/sip.conf.sample <span style="color: grey">(417210)</span></li>
<li>/branches/12/configs/pjsip.conf.sample <span style="color: grey">(417210)</span></li>
<li>/branches/12/configs/ari.conf.sample <span style="color: grey">(417210)</span></li>
<li>/branches/12/channels/sip/include/sip.h <span style="color: grey">(417210)</span></li>
<li>/branches/12/channels/chan_sip.c <span style="color: grey">(417210)</span></li>
<li>/branches/12/UPGRADE.txt <span style="color: grey">(417210)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3659/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>