| Summary: | ASTERISK-11090: Deadlock in pbx/sip introduced by fix for bug 0011323 | ||
| Reporter: | Ardjan Zwartjes (ardjan) | Labels: | |
| Date Opened: | 2007-12-20 09:41:34.000-0600 | Date Closed: | 2007-12-21 10:39:37.000-0600 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | PBX/General |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) 20071220__bug11609.diff.txt | |
| Description: | Bug 0011323 was fixed by locking &conlock in ast_hint_state_changed() (pbx.c). This does fix the mentioned problem, but introduces another possible deadlock: The function chain: sipsock_read()(chan_sip.c) -> handle_request()(chan_sip.c) -> handle_request_subscribe()(chan_sip.c) -> ast_extension_state()(pbx.c) -> ast_hint_extension() (pbx.c) first locks &p->lock (in find_call() called by sipsock_read()) and later on locks &conlock in ast_hint_extension(). The function chain: ast_hint_state_changed()(pbx.c) -> cb_extensionstate()(chan_sip.c) however now first locks &conlock in ast_hint_state_changed() and then locks &p->lock in cb_extensionstate(). This results in the situation where, if a device changes state and another device resubscribes at the same moment, a deadlock can occur (&p->lock will be the same mutex in both paths in this case). This situation already occurred on one of our servers where subscribes and notifies are used heavily. As far as I can see this problem is fixed in trunk by using read locks, I guess this would also be nice in the 1.4 branch but I can't really see the consequences of such a change. | ||
| Comments: | By: Olle Johansson (oej) 2007-12-21 04:49:45.000-0600 Thanks for a good bug report. We need to find a way to avoid this situation. By: Digium Subversion (svnbot) 2007-12-21 10:34:33.000-0600 Repository: asterisk Revision: 94466 U branches/1.4/include/asterisk/pbx.h U branches/1.4/main/pbx.c ------------------------------------------------------------------------ r94466 | russell | 2007-12-21 10:34:32 -0600 (Fri, 21 Dec 2007) | 6 lines Convert the contexts lock to a read/write lock to resolve a deadlock. This has a nice side benefit of improving performance. :) (closes issue ASTERISK-11090) (closes issue ASTERISK-10613) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=94466 By: Digium Subversion (svnbot) 2007-12-21 10:39:37.000-0600 Repository: asterisk Revision: 94467 _U trunk/ ------------------------------------------------------------------------ r94467 | russell | 2007-12-21 10:39:36 -0600 (Fri, 21 Dec 2007) | 13 lines Blocked revisions 94466 via svnmerge ........ r94466 | russell | 2007-12-21 10:37:47 -0600 (Fri, 21 Dec 2007) | 6 lines Convert the contexts lock to a read/write lock to resolve a deadlock. This has a nice side benefit of improving performance. :) (closes issue ASTERISK-11090) (closes issue ASTERISK-10613) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=94467 | ||