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



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

    So, while locking may solve the issue, there's something more insidious 
about this part of the code that doesn't sit well with me. (Note: this is the 
actual culprit that causes a crash in the websocket write)
    
    I'm not sure that the way this is currently handled is the right way to 
handle a AST_WEBSOCKET_OPCODE_CLOSE. The session destructor will already close 
the the file descriptor. Ideally, we'd just let the destruction of the session 
do this work for us.
    
    It feels like the right way to handle this may be to just let the caller of 
ast_websocket_read know that they were told that the session needs to die. That 
would let them de-ref the session appropriately. If a concurrent write was 
occurring at the same time, when the write completes, the session would be 
terminated.
    
    Now, whether or not it's allowed to have a write complete when you've just 
been told to close the websocket is another question. If not, then we have to 
keep all of the locking in here.


- Matt Jordan


On April 25, 2014, 1:46 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3481/
> -----------------------------------------------------------
> 
> (Updated April 25, 2014, 1:46 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23605
>     https://issues.asterisk.org/jira/browse/ASTERISK-23605
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This resolves a race condition where data could be written to a NULL FILE 
> pointer causing a crash as a websocket connection was in the process of 
> shutting down by adding locking to accesses and modifications of the 
> websocket session struct.
> 
> 
> Diffs
> -----
> 
>   branches/11/res/res_http_websocket.c 413007 
> 
> Diff: https://reviewboard.asterisk.org/r/3481/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> opticron
> 
>

-- 
_____________________________________________________________________
-- 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