Debugging some sip connections found that a server could get stuck with a partially unread buffer when the remote closed the connection. This could mean it was also not responding to signals to shut down. Reworked the main loop to be a bit more careful about checking the connection status. Moved it from the Sip.pm module into the server where it seemed to belong more logically. Have been running this with some other modifications in production sites for some time and appears to have enhanced reliability and ability to control the server.
Created attachment 36715 [details] [review] Proposed Patch
Created attachment 52111 [details] [review] [SIGNED-OFF] Bug 13807 Rework main input loop in SIPServer Debugging various problems in SIPServer and control of it, found it could loop on unread buffers (e.g. the LF of a CRLF if it was only expecting CR) making it unresponsive to signals. Reworked the input loop with an eye to removing unnecessary whiles and replacing the while(1) by a while( connection valid) Enhanced the timeout code by wapping in an eval. Moved the logic from SIP_read_packet into the server itself Hopefully this makes the already baroque code easier to navigate and it did seem the server was the logical place for this Removed no longer iused SIP_read_packet from Sip.pm Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Colin, how does this patch work with bug 15006? At the moment they seem to have merge conflicts.
Created attachment 52854 [details] [review] Bug 13807 Rework main input loop in SIPServer Debugging various problems in SIPServer and control of it, found it could loop on unread buffers (e.g. the LF of a CRLF if it was only expecting CR) making it unresponsive to signals. Reworked the input loop with an eye to removing unnecessary whiles and replacing the while(1) by a while( connection valid) Enhanced the timeout code by wapping in an eval. Moved the logic from SIP_read_packet into the server itself Hopefully this makes the already baroque code easier to navigate and it did seem the server was the logical place for this Removed no longer iused SIP_read_packet from Sip.pm Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Passing QA provisionally, we should deal with the interaction with bug 13807 before anything is pushed.
(In reply to Kyle M Hall from comment #5) > Passing QA provisionally, we should deal with the interaction with bug 13807 > before anything is pushed. 13807->15006 I rebased 15006 on top of this one. So it can go first..
(In reply to Marcel de Rooy from comment #6) > (In reply to Kyle M Hall from comment #5) > > Passing QA provisionally, we should deal with the interaction with bug 13807 > > before anything is pushed. > 13807->15006 > I rebased 15006 on top of this one. So it can go first.. I'm trying to add to 15006 a solution to the log in bug I identified, I'll generated that on top of this
Pushed to master for 16.11, thanks Colin!
This has been pushed to 16.05.x (August 1st), will be in 16.05.02
Patch pushed to 3.22.x, will be in 3.22.10