[asterisk-dev] mmichelson: trunk r157427 - /trunk/channels/chan_sip.c

[Mark Michelson]

Russell Bryant wrote:
> SVN commits to the Digium repositories wrote:
>> Author: mmichelson
>> Date: Tue Nov 18 14:23:58 2008
>> New Revision: 157427
>>
>> URL: http://svn.digium.com/view/asterisk?view=rev&rev=157427
>> Log:
>> * Add a lock to be used in the update_call_counter function.
>> * Revert logic to mirror 1.4's in the sense that it will not allow
>>   the call counter to dip below 0.
>>
>> These two measures prevent potential races that could cause a SIP peer
>> to appear to be busy forever.
>>
>> (closes issue #13668)
>> Reported by: mjc
>> Patches:
>>       hintfix_trunk_rev152649.patch uploaded by wolfelectronic (license 586)
>>
>>
>> Modified:
>>     trunk/channels/chan_sip.c
>>
>> Modified: trunk/channels/chan_sip.c
>> URL: http://svn.digium.com/view/asterisk/trunk/channels/chan_sip.c?view=diff&rev=157427&r1=157426&r2=157427
>> ==============================================================================
>> --- trunk/channels/chan_sip.c (original)
>> +++ trunk/channels/chan_sip.c Tue Nov 18 14:23:58 2008
>> @@ -2118,6 +2118,9 @@
>>  	SDP_T38_ACCEPT,   /*!< Remote side accepted our T38 request */
>>  };
>>  
>> +/*! \brief Protect the callcounters inuse,inringing and the corresponding flags */
>> +AST_MUTEX_DEFINE_STATIC(callctrlock);
>> +
> 
> What is the logic behind using this new global lock for this purpose? 
> What is wrong with just ensuring that the sip_pvt is locked during these 
> operations?
> 

The reason is that we are protecting boht flags that are in the sip_pvt 
structure and more importantly counters that are in the sip_peer structure (and 
in the case of 1.6.0, the sip_user structure, too). Because sip_peers and 
sip_pvts do not necessarily share a 1:1 relationship, the data in the sip_peer 
is not sufficiently protected by just locking the sip_pvt.

Mark Michelson