[Asterisk-code-review] res http websocket.c: prevent avoidable disconnections cause... (asterisk[11])

[Dade Brandon]

Dade Brandon has posted comments on this change.

Change subject: res_http_websocket.c: prevent avoidable disconnections caused by write errors
......................................................................


Patch Set 1:

Hi Matt,
Thank you for the feedback, I've corrected the code standard violations in the second patch set and will pay more attention to these in the future.

Regarding the close due to !session->f, I've removed that entire control block from the second patch set - I forgot that we'd added that assertion in separately from the changes referenced in the commit notes I submitted.  We saw a couple core dumps a long time ago with SIGSEGV due to null pointer dereference on session->f in this part of the code, but to correct it by checking for session->f here & by rewriting ast_websocket_close, would only serve to mask the source of the issue (and I've seen past reviews where to do so was advised against), so I'll keep it as an internal patch & hopefully someone will open an issue with a relevant backtrace so that the source of the null pointer can be identified, if it is still a problem.  I'm surprised that we didn't start seeing crashes in ast_websocket_close after patching that, so maybe someone else patched the source of that problem around the same time as us dealing with it in ast_websocket_write.

-- 
To view, visit https://gerrit.asterisk.org/1863
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4db7a586af1c7a57184c31d3d55bf146f1a40598
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Dade Brandon <dade at xencall.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Dade Brandon <dade at xencall.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: No