[asterisk-dev] [Code Review] 4431: Increase WebSocket frame size and improve large read handling

[rmudgett]

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



/branches/11/res/res_http_websocket.c
<https://reviewboard.asterisk.org/r/4431/#comment25042>

    Change this line to:
    sanity = 10;
    for (;;) {



/branches/11/res/res_http_websocket.c
<https://reviewboard.asterisk.org/r/4431/#comment25043>

    Change to:
    if (!--sanity) {
      move after loop sanity check closing here.
    }
    
    Moving the sanity exit handling here will avoid waiting (possibly for a long time) for data when we have already failed sanity and it won't matter if we somehow actually do read more data.
    
    The loop becomes:
    sanity = 10;
    for (;;) {
      ...
      if (!rlen) {
        ...
        if (!--sanity) {
           sanity session close and exit.
        }
      }
    }
    return 0;



/branches/11/res/res_http_websocket.c
<https://reviewboard.asterisk.org/r/4431/#comment25044>

    Move to were sanity is now decremented.  See above issue location.


- rmudgett


On Feb. 18, 2015, 4:32 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4431/
> -----------------------------------------------------------
> 
> (Updated Feb. 18, 2015, 4:32 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Some WebSocket applications, like [chan_respoke][], require a larger
> frame size than the default 8k; this patch bumps the default to 16k.
> This patch also fixes some problems exacerbated by large frames.
> 
> The sanity counter was decremented on every fread attempt in
> ws_safe_read(), regardless of whether data was read from the socket or
> not. For large frames, this could result in loss of sanity prior to
> reading the entire frame. (16k frame / 1448 bytes per segment = 12
> segments).
> 
> This patch changes the sanity counter so that it only decrements when
> fread() doesn't read any bytes. This more closely matches the original
> intention of ws_safe_read(), given that the error message is
> "Websocket seems unresponsive".
> 
> This patch also properly logs EOF conditions, so disconnects are no
> longer confused with unresponsive connections.
> 
>  [chan_respoke]: https://github.com/respoke/chan_respoke
> 
> 
> Diffs
> -----
> 
>   /branches/11/res/res_http_websocket.c 431915 
> 
> Diff: https://reviewboard.asterisk.org/r/4431/diff/
> 
> 
> Testing
> -------
> 
> Ran a Node app that continuously send large WebSocket frame to Asterisk.
> 
> https://gist.github.com/leedm777/ba6d86468d7646073286
> 
> Without the patch, Asterisk fails in less than 10 frames. With the patch, it runs like a boss.
> 
> 
> Thanks,
> 
> David Lee
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150220/723add2f/attachment-0001.html>