----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3548/#review11978 -----------------------------------------------------------
/trunk/channels/chan_dahdi.c <https://reviewboard.asterisk.org/r/3548/#comment21890> Guidelines. Curly on next line for function. /trunk/channels/chan_dahdi.c <https://reviewboard.asterisk.org/r/3548/#comment21894> Code the function this way so you don't have to access list members directly. static void release_doomed_pris(void) { #if defined(HAVE_PRI) struct doomed_pri *node; AST_LIST_LOCK(&doomed_pris); while ((node = AST_LIST_REMOVE_HEAD(&doomed_pris, list)) { AST_LIST_UNLOCK(&doomed_pris); pri_destroy_span(node->pri); ast_free(node); AST_LIST_LOCK(&doomed_pris); } AST_LIST_UNLOCK(&doomed_pris); #endif /* defined(HAVE_PRI) */ } /trunk/channels/chan_dahdi.c <https://reviewboard.asterisk.org/r/3548/#comment21891> Guidelines. Curly on next line for function. /trunk/channels/sig_pri.c <https://reviewboard.asterisk.org/r/3548/#comment21892> Put this in its own wrapper function like all the others and test if the function pointer is NULL before calling. - rmudgett On May 19, 2014, 9:32 a.m., Tzafrir Cohen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3548/ > ----------------------------------------------------------- > > (Updated May 19, 2014, 9:32 a.m.) > > > Review request for Asterisk Developers and rmudgett. > > > Bugs: ASTERISK-23554 > https://issues.asterisk.org/jira/browse/ASTERISK-23554 > > > Repository: Asterisk > > > Description > ------- > > Issue: when a PRI span is disconnected (e.g.: following the unassignment pri > spans) dahdi channels of that span can be destroyed in two different pathes: > > 1. DAHDI channels are destroyed in response to pri_event_removed > 2. The span is destroyed in response to DAHDI_EVENT_REMOVED in the D-channel. > Before the span is destroyed, its channels need to be destroyed. > > If the channel is not in a call, (1) is run from the monitor thread, holding > the iflock (lock of iflist: the list of channels). somewhere in the process > of destroying a channel that belongs to a PRI > span, the pri's lock needs to be acquired. > > (2) is called from a context of handling the PRI events and hence holds the > PRI lock. Destroying the channels requires getting the iflock. > > Which means that if the two happen simultaneously, we have a deadlock. And > the two will happen simultaneously, as recent versions of DAHDI will send an > extra DAHDI_EVENT_REMOVED as a response to any call to the ioctl on > DAHDI_GET_EVENT on a removed span. > > This review includes the patches pri_destroy_span_prilist.patch and > sigpri_handle_enodev_1.patch from the referred bug. The former solves this > deadlock by creating a list of spans to be removed "later" and and thus allow > executing (2) without holding the pri lock. > > The second patch fixes error handling of libpri: if read returns -ENODEV, we > have no device and it should be destroyed. This, however, requires exposing > the above "deferred destruction" functionality to sig_pri. > > > Diffs > ----- > > /trunk/channels/sig_pri.c 414151 > /trunk/channels/sig_pri.h 414151 > /trunk/channels/chan_dahdi.c 414151 > > Diff: https://reviewboard.asterisk.org/r/3548/diff/ > > > Testing > ------- > > > Thanks, > > Tzafrir Cohen > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
