[asterisk-dev] [Code Review] Allow transfer loops w/o allowing forwarding loops

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1195/#review3421
-----------------------------------------------------------


I can explain why you were only finding the datastore on peer and not on chan. Look in app_dial, dial_exec_full(), just below the call to wait_for_answer(). You can see that we remove the datastore from chan at that point. Apparently, this situation you are fixing was foreseen, but not quite well enough. The issue is that the datastore gets inherited by the peer channel, but unfortunately, the datastore is still on the peer at the time of the bridge.

I'd suggest grouping the two removals together. Personally, I like the idea of placing the removal in ast_bridge_calls() where you have it now. If you do that, be forewarned you'll have to remove the datastore removal code from both app_dial and app_queue since both make use of this same datastore.

- Mark


On 2011-04-22 17:24:30, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1195/
> -----------------------------------------------------------
> 
> (Updated 2011-04-22 17:24:30)
> 
> 
> Review request for Asterisk Developers, Mark Michelson and David Vossel.
> 
> 
> Summary
> -------
> 
> We try to avoid the situation where two phones may be forwarded to each other causing an infinite loop by storing each dialed interface in a channel datastore and checking the list before dialing out. This works, but currently breaks situations like A calls B, A transfers B to C, B transfers C to A, and A transfers C to B. Since human interaction is happening here and not an automated forwarding loop, it should be allowed.
> 
> This patch removes the dialed_interfaces datastore when a call is bridged (a suggestion from the brilliant mmichelson). If a call is being bridged, it should be safe to assume that we aren't stuck in a loop.
> 
> I determined that it was "peer" and not "chan" where the datastore was stored by trial and error. No matter what I tried, "chan" never had the datastore. I will fully admit that the features code still confuses the hell out of me even after all of these years.
> 
> Note: This bug exists also up to 1.8 (probably trunk, I just haven't tried yet)
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/res/res_features.c 314725 
> 
> Diff: https://reviewboard.asterisk.org/r/1195/diff
> 
> 
> Testing
> -------
> 
> I tried the above scenario and it now works. I also tried forwarding a couple of phones to each other and it still broke out of the loop.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110422/42b005ab/attachment-0001.htm>