| Summary: | ASTERISK-10837: ast_hint_state_changed locks conlock / hints in wrong order when monitoring SIP channels | ||
| Reporter: | Eelco Brolman (eelcob) | Labels: | |
| Date Opened: | 2007-11-20 07:29:13.000-0600 | Date Closed: | 2007-11-20 11:56:43.000-0600 |
| Priority: | Minor | Regression? | No |
| Status: | Closed/Complete | Components: | PBX/General |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ||
| Description: | ast_hint_state_changed locks the hints lock and calls the callback functions for the extensions. In case of a SIP extensions, the following chain is followed: "cb_extensionstate" (chan_sip.c) -> "transmit_state_notify" (chan_sip.c) -> "ast_get_hint" (pbx.c) -> "ast_hint_extension" (pbx.c) In the last function, the conlock is also claimed. This conflicts with (quoting comment above static AST_LIST_HEAD_STATIC(hints, ast_hint);): WARNING: When holding this list's lock, do _not_ do anything that will cause conlock to be taken, unless you _already_ hold it. The ast_merge_contexts_and_delete function will take the locks in conlock/hints order, so any other paths that require both locks must also take them in that order. This *can* (obviously) results in a deadlock. Easy to reproduce when inserting a slight delay (sleep(10);) between the locking of conlock and hints in ast_merge_contexts_and_delete and reloading asterisk when monitoring state changes of SIP devices. ****** ADDITIONAL INFORMATION ****** Solution: lock the conlock in the function ast_hint_state_changed before locking the hints (and unlock at the end). As far as we have seen, this has no side-effects, but we have not the in-depth knowledge of the hint system. | ||
| Comments: | By: Digium Subversion (svnbot) 2007-11-20 11:48:15.000-0600 Repository: asterisk Revision: 89457 U branches/1.4/main/pbx.c ------------------------------------------------------------------------ r89457 | mmichelson | 2007-11-20 11:48:14 -0600 (Tue, 20 Nov 2007) | 9 lines According to comments in main/pbx.c, it is essential that if we are going to lock the conlock as well as the hints lock, it must be locked in that respective order. In order to prevent a potential deadlock, we need to lock the conlock prior to locking the hints lock in ast_hint_state_changed (see the call stack example on issue ASTERISK-10837 for how this can happen). (closes issue ASTERISK-10837, reported by eelcob, suggestion for patch by eelcob, patch by me) ------------------------------------------------------------------------ By: Digium Subversion (svnbot) 2007-11-20 11:56:43.000-0600 Repository: asterisk Revision: 89458 _U trunk/ U trunk/main/pbx.c ------------------------------------------------------------------------ r89458 | mmichelson | 2007-11-20 11:56:43 -0600 (Tue, 20 Nov 2007) | 17 lines Merged revisions 89457 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........ r89457 | mmichelson | 2007-11-20 11:50:31 -0600 (Tue, 20 Nov 2007) | 9 lines According to comments in main/pbx.c, it is essential that if we are going to lock the conlock as well as the hints lock, it must be locked in that respective order. In order to prevent a potential deadlock, we need to lock the conlock prior to locking the hints lock in ast_hint_state_changed (see the call stack example on issue ASTERISK-10837 for how this can happen). (closes issue ASTERISK-10837, reported by eelcob, suggestion for patch by eelcob, patch by me) ........ ------------------------------------------------------------------------ | ||