[asterisk-dev] Pre-requesites of ast_cel_report_event breached in handle_request_refer (non-permitted lock)

[Dave WOOLLEY]

Walter Doekes <walter+asterisk-dev at osso.nl> wrote:

> AFAICT, local_attended_transfer does that as well (locked by

[[djw]] 
And it looks like handle_invite_replaces does, although the place I've found has both relevant channels locked, so may be safe.

> owner_chan_ref = sip_pvt_lock_full(p) in handle_request_do or by
> get_sip_pvt_byid_locked, or both):
> 
> > /* both p and p->owner _MUST_ be locked while calling
> > local_attended_transfer */ if ((res = local_attended_transfer(p,
> > &current, req, seqno, nounlock))) {
> ...
> > /* target.chan1 was locked in get_sip_pvt_byid_locked, do not unlock
> > target.chan1 before this */ ast_cel_report_event(target.chan1,
> > AST_CEL_ATTENDEDTRANSFER, NULL, transferer_linkedid, target.chan2);
> 
> And that in turn causes a deadlock when the AST_CEL_BRIDGE_UPDATE
> event is simultaneously fired by a different thread (ast_do_masquerade).
> 
> We've had to disable BRIDGE_UPDATE cel events for now as a workaround.
> 

Is there a formal issue report for this?  I was reluctant to raise one because it was picked up reading the code of a version we don't use, rather than in anger.  It seems that you've actually identified a specific real world race condition.

Also, it seems to me that one could avoid locking issues by passing addresses in the event, and having the accounting software, which is presumably single thread, match those up.  If a channel name is actually needed, for a peer,  rather than just an identifier for the structure, couldn't the accounting software look that up.  In that, case, if anything the channel should be locked, either on entry, or immediately after entry, to ensure that masquerading, etc., didn't change anything between when the event data was collected and when the event was queued, so that the events are guaranteed to be in a causally consistent order. 
-- 
David Woolley
BTS Holdings Plc
Tel: +44 (0)20 8401 9000 Fax: +44 (0)20 8401 9100
http://www.bts.co.uk

BTS Holdings PLC - Registered office: BTS House, Manor Road, Wallington, SM6 0DD - Registered in England: 1517630