[asterisk-dev] [Code Review] Fix FRACK message caused by chan_iax2

[David Vossel]

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


The chan_iax parts of this fix look correct to me.  We should be setting the lagid = -1 during the schedule cb instead of the function scheduled by another thread, other wise we could potentially attempt to remove that schedule id after it is already gone.

My only comment about chan_iax is since the lagid is set to -1 in send_lagrq, then it really doesn't need to get set to -1 again in __send_lagrq.

I do question the changes in sched.h.  Why is it necessary to change the sleep time, and why was it so high to begin with?

- David


On 2010-08-13 10:27:47, Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/861/
> -----------------------------------------------------------
> 
> (Updated 2010-08-13 10:27:47)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> The FRACK message out of the scheduler indicates that we attempted to cancel an ID that does not exist.  This usually indicates a race condition.
> 
> What I've found is that due to the way that we schedule multiple threads in chan_iax2 to run scheduled tasks, there can be an extended period of time that a scheduled task has been successfully run by the scheduler, yet the task has not actually run, and therefore, has not altered the stored scheduler ID.  When teardown attempts to cancel all scheduled tasks, the scheduler complains and shortly thereafter, the task complains that the callno does not exist.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_iax2.c 282013 
>   /branches/1.8/include/asterisk/sched.h 282013 
> 
> Diff: https://reviewboard.asterisk.org/r/861/diff
> 
> 
> Testing
> -------
> 
> Since this is a race condition, it's difficult to test.  However, I have run with this patch for extended periods (i.e. overnight) that would have previously caused the FRACK message to appear several times, and the FRACK message did not appear.
> 
> 
> Thanks,
> 
> Tilghman
> 
>