Review Board 1.7.16


Improve timing interface to remember which provider provided a timer

Review Request #211 - Created March 26, 2009 and submitted

Kevin Fleming
trunk
14697
Reviewers
asterisk-dev
Asterisk
The ability to load/unload timing interfaces is nice, but it means that when a timer is allocated, it may come from provider A, but later provider B becomes the 'preferred' provider. If this happens, all timer API calls on the timer that was provided by provider A will actually be handed to provider B, which will say WTF and return an error.

This patch changes the timer API to include a pointer to the provider of the timer handle so that future operations on the timer will be forwarded to the proper provider.
Compile testing only.
Posted (March 26, 2009, 3:39 p.m.)

   

  
/trunk/channels/chan_iax2.c (Diff revision 1)
 
 
I think this should be 'static'
  1. That is correct; I'll fix it during the commit.
Ship it!
Posted (March 27, 2009, 3:41 a.m.)
Pending testing and the one minor issue I found, this looks good to me.
/trunk/bridges/bridge_softmix.c (Diff revision 1)
 
 
 
 
 
 
 
 
 
 
 
 
 
 
The changes here look fine.  These comments are actually for Josh, since it highlighted this bit of code.

Josh, this is an interesting function.  I presume the point is to ensure that a timing interface is available before bridge_softmix gets used.  However, while this test verifies that a timing interface is available at this instant, it does not ensure that a timing interface will _continue_ to be available.

All you need to do to remove this (admittedly theoretical, and highly unlikely) race condition, is not close this timer until the bridge is torn down.  Perhaps you could just add a void *tech_pvt to the ast_bridge struct.  It could be potentially useful for other things down the road, as well. 
  1. Ooh good idea, I didn't think to use bridge_pvt like that. Done.
/trunk/bridges/bridge_softmix.c (Diff revision 1)
 
 
 
Again, actually a comment for Josh.  :-)

Is there a reason you need to set the rate back to 0 here when you're about to close the timer anyway?
  1. Nope, fixed.
/trunk/main/channel.c (Diff revision 1)
 
 
 
 
 
 
If a timer is not allocated, tmp->timingfd should be set to -1.  There is some code that uses that value to check to see if it is valid.  (ast_channel_set_fd() specifically, maybe other places ..)

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.