-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3659/#review12318
-----------------------------------------------------------

Ship it!



/branches/12/res/res_http_websocket.c
<https://reviewboard.asterisk.org/r/3659/#comment22494>

    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)) {
        ...
    }


- Mark Michelson


On June 25, 2014, 6:38 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3659/
> -----------------------------------------------------------
> 
> (Updated June 25, 2014, 6:38 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23917
>     https://issues.asterisk.org/jira/browse/ASTERISK-23917
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> *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.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip_transport_websocket.c 417210 
>   /branches/12/res/res_pjsip/config_transport.c 417210 
>   /branches/12/res/res_pjsip.c 417210 
>   /branches/12/res/res_http_websocket.c 417210 
>   /branches/12/res/res_ari.c 417210 
>   /branches/12/res/ari/internal.h 417210 
>   /branches/12/res/ari/config.c 417210 
>   /branches/12/res/ari/ari_websockets.c 417210 
>   /branches/12/include/asterisk/res_pjsip.h 417210 
>   /branches/12/include/asterisk/http_websocket.h 417210 
>   /branches/12/configs/sip.conf.sample 417210 
>   /branches/12/configs/pjsip.conf.sample 417210 
>   /branches/12/configs/ari.conf.sample 417210 
>   /branches/12/channels/sip/include/sip.h 417210 
>   /branches/12/channels/chan_sip.c 417210 
>   /branches/12/UPGRADE.txt 417210 
> 
> Diff: https://reviewboard.asterisk.org/r/3659/diff/
> 
> 
> Testing
> -------
> 
> 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).
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to