| Summary: | ASTERISK-13806: [patch] lock or crash after changing sip 'transport' | ||
| Reporter: | pj (pj) | Labels: | |
| Date Opened: | 2009-03-23 16:59:15 | Date Closed: | 2009-05-22 17:51:12 |
| Priority: | Major | Regression? | No |
| Status: | Closed/Complete | Components: | Channels/chan_sip/General |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) coredebug.txt ( 1) gdb.txt ( 2) gdb2.txt ( 3) lock2.txt ( 4) reg_patch_3.diff ( 5) show-locks.txt ( 6) sip.conf ( 7) sipdump.pcap | |
| Description: | When I change sip transport protocol order, eg. udp,tcp -> tcp,udp and do 'sip reload' asterisk locks or crash | ||
| Comments: | By: pj (pj) 2009-03-23 17:08:41 I think also, that current design, when asterisk doesn't use same transport for outgoing request as peer is using for registration, is bad, because eg. 'qualify' always marks peer as unreachable, in case when peer registers eg. with tcp, but asterisk has configured transport=udp,tcp imho, better behaviour will be to use same transport, that peer is using for registration, regardless of order of 'transport' protocol fields in sip.conf By: pj (pj) 2009-03-24 02:54:13 issues with "multiple transport" was also solved in bugreport ASTERISK-1303117 but patches supplied there caused another problems and bugreport has no activity long time. By: David Vossel (dvossel) 2009-04-16 10:05:06 I've taken a brief look at this issue but have not been able to reproduce the deadlock. I've got a few more ways I'd like to test it, but it would be helpful to have more information as to what the system is doing before the deadlock. What kind of environment does it need to be in for this to happen? Also, is this something that happens %100 of the time or just occasionally? By: pj (pj) 2009-04-16 10:38:51 I can easy reproduce this lock or crash. Is sufficient to put transport=tcp,udp in sip.conf and try to register (I'm using eyebeam softphone). you can try experiment with attached sip.conf By: David Vossel (dvossel) 2009-04-20 16:47:22 why do you have tcpenable=no, but then set transport to be tcp,udp? By: Digium Subversion (svnbot) 2009-04-21 15:28:38 Repository: asterisk Revision: 189771 U trunk/channels/chan_sip.c ------------------------------------------------------------------------ r189771 | dvossel | 2009-04-21 15:28:38 -0500 (Tue, 21 Apr 2009) | 11 lines Fixes segfault when switching upd to tcp in sip.conf after reload. If transport in sip.conf is switched from upd to tcp, Asterisk segfaults right after issuing a sip reload. The problem is the socket type is changed to TCP but the fd may still be present for UDP. Later, when the tcp session should be created or set using an existing one, it isn't because the old file descriptor is still present. Now every time transport is changed during a sip.conf reload, the file descriptor is set to -1, signifying it must be created or found. (closes issue ASTERISK-13806) Reported by: pj Tested by: dvossel Review: http://reviewboard.digium.com/r/229/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=189771 By: Digium Subversion (svnbot) 2009-04-21 15:34:40 Repository: asterisk Revision: 189771 U trunk/channels/chan_sip.c ------------------------------------------------------------------------ r189771 | dvossel | 2009-04-21 15:28:37 -0500 (Tue, 21 Apr 2009) | 11 lines Fixes segfault when switching UDP to TCP in sip.conf after reload. If transport in sip.conf is switched from upd to tcp, Asterisk segfaults right after issuing a sip reload. The problem is the socket type is changed to TCP but the fd may still be present for UDP. Later, when the tcp session should be created or set using an existing one, it isn't because the old file descriptor is still present. Now every time transport is changed during a sip.conf reload, the file descriptor is set to -1, signifying it must be created or found. (closes issue ASTERISK-13806) Reported by: pj Tested by: dvossel Review: http://reviewboard.digium.com/r/229/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=189771 By: Digium Subversion (svnbot) 2009-04-21 15:38:02 Repository: asterisk Revision: 189771 U trunk/channels/chan_sip.c ------------------------------------------------------------------------ r189771 | dvossel | 2009-04-21 15:28:37 -0500 (Tue, 21 Apr 2009) | 11 lines Fixes segfault when switching UDP to TCP in sip.conf after reload. If transport in sip.conf is switched from UDP to TCP, Asterisk segfaults right after issuing a sip reload. The problem is the socket type is changed to TCP but the fd may still be present for UDP. Later, when the TCP session should be created or set using an existing one, it isn't because the old file descriptor is still present. Now every time transport is changed during a sip.conf reload, the file descriptor is set to -1, signifying it must be created or found. (closes issue ASTERISK-13806) Reported by: pj Tested by: dvossel Review: http://reviewboard.digium.com/r/229/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=189771 By: Digium Subversion (svnbot) 2009-04-21 15:41:42 Repository: asterisk Revision: 189773 _U branches/1.6.0/ U branches/1.6.0/channels/chan_sip.c ------------------------------------------------------------------------ r189773 | dvossel | 2009-04-21 15:41:42 -0500 (Tue, 21 Apr 2009) | 17 lines Merged revisions 189771 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r189771 | dvossel | 2009-04-21 15:28:37 -0500 (Tue, 21 Apr 2009) | 11 lines Fixes segfault when switching UDP to TCP in sip.conf after reload. If transport in sip.conf is switched from UDP to TCP, Asterisk segfaults right after issuing a sip reload. The problem is the socket type is changed to TCP but the fd may still be present for UDP. Later, when the TCP session should be created or set using an existing one, it isn't because the old file descriptor is still present. Now every time transport is changed during a sip.conf reload, the file descriptor is set to -1, signifying it must be created or found. (closes issue ASTERISK-13806) Reported by: pj Tested by: dvossel Review: http://reviewboard.digium.com/r/229/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=189773 By: Digium Subversion (svnbot) 2009-04-21 15:42:56 Repository: asterisk Revision: 189774 _U branches/1.6.1/ U branches/1.6.1/channels/chan_sip.c ------------------------------------------------------------------------ r189774 | dvossel | 2009-04-21 15:42:55 -0500 (Tue, 21 Apr 2009) | 17 lines Merged revisions 189771 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r189771 | dvossel | 2009-04-21 15:28:37 -0500 (Tue, 21 Apr 2009) | 11 lines Fixes segfault when switching UDP to TCP in sip.conf after reload. If transport in sip.conf is switched from UDP to TCP, Asterisk segfaults right after issuing a sip reload. The problem is the socket type is changed to TCP but the fd may still be present for UDP. Later, when the TCP session should be created or set using an existing one, it isn't because the old file descriptor is still present. Now every time transport is changed during a sip.conf reload, the file descriptor is set to -1, signifying it must be created or found. (closes issue ASTERISK-13806) Reported by: pj Tested by: dvossel Review: http://reviewboard.digium.com/r/229/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=189774 By: Digium Subversion (svnbot) 2009-04-21 15:45:45 Repository: asterisk Revision: 189775 _U branches/1.6.2/ U branches/1.6.2/channels/chan_sip.c ------------------------------------------------------------------------ r189775 | dvossel | 2009-04-21 15:45:45 -0500 (Tue, 21 Apr 2009) | 17 lines Merged revisions 189771 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r189771 | dvossel | 2009-04-21 15:28:37 -0500 (Tue, 21 Apr 2009) | 11 lines Fixes segfault when switching UDP to TCP in sip.conf after reload. If transport in sip.conf is switched from UDP to TCP, Asterisk segfaults right after issuing a sip reload. The problem is the socket type is changed to TCP but the fd may still be present for UDP. Later, when the TCP session should be created or set using an existing one, it isn't because the old file descriptor is still present. Now every time transport is changed during a sip.conf reload, the file descriptor is set to -1, signifying it must be created or found. (closes issue ASTERISK-13806) Reported by: pj Tested by: dvossel Review: http://reviewboard.digium.com/r/229/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=189775 By: pj (pj) 2009-05-04 10:06:01 sad to say, issue still persist, I have tcpenable=yes in global and transport=tcp,udp in friend definition section It locks immediatelly after phone (xlite/eyebeam) registers. uploaded lock2.txt and gdb2.txt Asterisk SVN-trunk-r191136 By: David Vossel (dvossel) 2009-05-04 16:26:45 Sorry my last patch didn't fully resolve the issue. Thanks for the bt and gdb info, would it be possible to get a packet capture as well as debug output of when this is happening? By: pj (pj) 2009-05-05 03:47:38 uploaded coredebug.txt that contains output from "core set verbose 3" and "sip set debug on" and relevant packet capture - sipdump.pcap By: pj (pj) 2009-05-05 03:56:01 side note, asterisk is trying to send 'options' request to phone using TCP, but it's wrong, because phone is registered using UDP. Asterisk should use transport protocol, that phone actualy uses in registration request and not rely on transport order specified by "transport=tcp,udp" in this case. By: David Vossel (dvossel) 2009-05-07 18:04:35 Does the lock you reopened this issue for still require a sip reload? If so, check out the patch I uploaded. It may or may not help you, but it fixes something I missed in my original fix. If it doesn't require a sip reload to reproduce this let me know. We may need to open a new report for it since its different from the initial one. By: pj (pj) 2009-05-08 15:38:28 it locks if I have phone registered with SIP/UDP and I will do "sip reload" after change transport=udp,tcp -> transport=tcp,udp Asterisk SVN-trunk-r193274 + tcp_reload_fix.diff === === Thread ID: -1217205360 (do_monitor started at [22050] chan_sip.c restart_monitor()) === ---> Lock #0 (chan_sip.c): MUTEX 22022 do_monitor &monlock 0xb7828340 (1) asterisk(ast_bt_get_addresses+0x19) [0x8114fe9] /usr/lib/asterisk/modules/chan_sip.so [0xb779bb37] /usr/lib/asterisk/modules/chan_sip.so [0xb77fc3e2] asterisk [0x818bf32] /lib/i686/libpthread.so.0 [0xb7b96315] /lib/i686/libc.so.6(clone+0x5e) [0xb7e1525e] === ------------------------------------------------------------------- By: David Vossel (dvossel) 2009-05-14 10:38:19 take a look at reg_patch_1.diff. I've reworked the way the transport option works in SIP. Now if the transport = tcp,udp, tcp is used as the default for outbound connections only until a Registration takes place. If during the registration the device specifies udp, then udp will be used for the outbound connection instead of tcp. By: pj (pj) 2009-05-14 14:15:25 thanks for patch! I found, that prefered transport protocol isn't working as expected eg: I have transport=udp,tcp register phone using tcp asterisk continues using tcp as primary transport, even if phone is unregistered (it is shown in "sip show peer xxx" - Prim.Transp.) I must do "sip reload" or restart asterisk, that udp will be used again for outbound connections. By: David Vossel (dvossel) 2009-05-14 14:56:06 Okay, so I just need to make the peer go back to the original default transport if the registration expires? Does this resolve the issue you had with sip reload crashing? By: pj (pj) 2009-05-14 15:25:05 after phone is unregistered, asterisk should revert to use transport that is specified in sip.conf for particular peer, imho it's expected behaviour... and yes, locking issues are gone, thanks! By: David Vossel (dvossel) 2009-05-14 17:42:30 reg_patch_2.diff should do it. let me know if you have any problems. By: pj (pj) 2009-05-16 10:31:10 tested reg_patch_2.diff, but I don't see any change from my last testing, ie. asterisk doesn't revert to order of transport, that is specified in sip.conf, after phone is unregistered By: David Vossel (dvossel) 2009-05-21 11:58:09 reg_patch_3.diff should reset the default transport when a peer unregisters. The other patch only reset the default transport when the peer's registration expired. I thought this was the same logic that was used when a peer Unregistered, but I was wrong. Hopefully this new patch will be the last. Thanks for your feedback! By: pj (pj) 2009-05-22 10:12:44 reg_patch_3.diff is working fine, thank you for this great patch! tested with Asterisk SVN-trunk-r196072 By: David Vossel (dvossel) 2009-05-22 10:32:39 thanks for your feedback! I've got the patch up for review, it should be committed soon. By: Digium Subversion (svnbot) 2009-05-22 16:09:50 Repository: asterisk Revision: 196416 U trunk/channels/chan_sip.c U trunk/configs/sip.conf.sample ------------------------------------------------------------------------ r196416 | dvossel | 2009-05-22 16:09:45 -0500 (Fri, 22 May 2009) | 19 lines SIP set outbound transport type from Registration In sip.conf the transport option allows for the configuration of what transport types (udp, tcp, and tls) a peer will accept, but only the first type listed was used for outbound connections. This patch changes this. Now the default transport type is only used until the peer registers. When registration takes place the transport type is parsed out of the Contact header. If the Contact header's transport type is equal to one that the peer supports, the peer's default transport type for outbound connections is set to match the Contact header's type. If the Contact header's transport type is not present, then the peer's default transport type is set to match the one the peer registered with. When a peer unregisters or the registration expires, the default transport type for that peer is reset. (closes issue ASTERISK-11705) Reported by: rjain Patches: reg_patch_1.diff uploaded by dvossel (license 671) Tested by: dvossel (closes issue ASTERISK-13806) Reported by: pj Patches: reg_patch_3.diff uploaded by dvossel (license 671) Tested by: pj, dvossel Review: https://reviewboard.asterisk.org/r/249/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=196416 By: Digium Subversion (svnbot) 2009-05-22 16:59:34 Repository: asterisk Revision: 196452 _U branches/1.6.2/ U branches/1.6.2/channels/chan_sip.c U branches/1.6.2/configs/sip.conf.sample ------------------------------------------------------------------------ r196452 | dvossel | 2009-05-22 16:59:32 -0500 (Fri, 22 May 2009) | 25 lines Merged revisions 196416 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r196416 | dvossel | 2009-05-22 16:09:45 -0500 (Fri, 22 May 2009) | 19 lines SIP set outbound transport type from Registration In sip.conf the transport option allows for the configuration of what transport types (udp, tcp, and tls) a peer will accept, but only the first type listed was used for outbound connections. This patch changes this. Now the default transport type is only used until the peer registers. When registration takes place the transport type is parsed out of the Contact header. If the Contact header's transport type is equal to one that the peer supports, the peer's default transport type for outbound connections is set to match the Contact header's type. If the Contact header's transport type is not present, then the peer's default transport type is set to match the one the peer registered with. When a peer unregisters or the registration expires, the default transport type for that peer is reset. (closes issue ASTERISK-11705) Reported by: rjain Patches: reg_patch_1.diff uploaded by dvossel (license 671) Tested by: dvossel (closes issue ASTERISK-13806) Reported by: pj Patches: reg_patch_3.diff uploaded by dvossel (license 671) Tested by: pj, dvossel Review: https://reviewboard.asterisk.org/r/249/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=196452 By: Digium Subversion (svnbot) 2009-05-22 17:35:49 Repository: asterisk Revision: 196453 _U branches/1.6.1/ U branches/1.6.1/channels/chan_sip.c U branches/1.6.1/configs/sip.conf.sample ------------------------------------------------------------------------ r196453 | dvossel | 2009-05-22 17:35:47 -0500 (Fri, 22 May 2009) | 25 lines Merged revisions 196416 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r196416 | dvossel | 2009-05-22 16:09:45 -0500 (Fri, 22 May 2009) | 19 lines SIP set outbound transport type from Registration In sip.conf the transport option allows for the configuration of what transport types (udp, tcp, and tls) a peer will accept, but only the first type listed was used for outbound connections. This patch changes this. Now the default transport type is only used until the peer registers. When registration takes place the transport type is parsed out of the Contact header. If the Contact header's transport type is equal to one that the peer supports, the peer's default transport type for outbound connections is set to match the Contact header's type. If the Contact header's transport type is not present, then the peer's default transport type is set to match the one the peer registered with. When a peer unregisters or the registration expires, the default transport type for that peer is reset. (closes issue ASTERISK-11705) Reported by: rjain Patches: reg_patch_1.diff uploaded by dvossel (license 671) Tested by: dvossel (closes issue ASTERISK-13806) Reported by: pj Patches: reg_patch_3.diff uploaded by dvossel (license 671) Tested by: pj, dvossel Review: https://reviewboard.asterisk.org/r/249/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=196453 By: Digium Subversion (svnbot) 2009-05-22 17:51:12 Repository: asterisk Revision: 196454 _U branches/1.6.0/ U branches/1.6.0/channels/chan_sip.c U branches/1.6.0/configs/sip.conf.sample ------------------------------------------------------------------------ r196454 | dvossel | 2009-05-22 17:51:10 -0500 (Fri, 22 May 2009) | 25 lines Merged revisions 196416 via svnmerge from https://origsvn.digium.com/svn/asterisk/trunk ........ r196416 | dvossel | 2009-05-22 16:09:45 -0500 (Fri, 22 May 2009) | 19 lines SIP set outbound transport type from Registration In sip.conf the transport option allows for the configuration of what transport types (udp, tcp, and tls) a peer will accept, but only the first type listed was used for outbound connections. This patch changes this. Now the default transport type is only used until the peer registers. When registration takes place the transport type is parsed out of the Contact header. If the Contact header's transport type is equal to one that the peer supports, the peer's default transport type for outbound connections is set to match the Contact header's type. If the Contact header's transport type is not present, then the peer's default transport type is set to match the one the peer registered with. When a peer unregisters or the registration expires, the default transport type for that peer is reset. (closes issue ASTERISK-11705) Reported by: rjain Patches: reg_patch_1.diff uploaded by dvossel (license 671) Tested by: dvossel (closes issue ASTERISK-13806) Reported by: pj Patches: reg_patch_3.diff uploaded by dvossel (license 671) Tested by: pj, dvossel Review: https://reviewboard.asterisk.org/r/249/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=196454 | ||