[asterisk-dev] mmichelson: trunk r157427 - /trunk/channels/chan_sip.c
mmichelson at digium.com
Tue Nov 18 15:46:31 CST 2008
Mark Michelson wrote:
> Russell Bryant wrote:
>> Mark Michelson wrote:
>>> Russell Bryant wrote:
>>>> SVN commits to the Digium repositories wrote:
>>>>> +/*! \brief Protect the callcounters inuse,inringing and the
>>>>> corresponding flags */
>>>> 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.
>> Ok, then it seems like the right thing is to have the sip_pvt and the
>> sip_peer locked, then.
> I thought about doing that, too, but I couldn't really see any added
> value in adding a lock to the sip_peer structure over the solution that
> was presented in the provided patch. I suppose that since we are
> protecting data that is part of the sip_peer structure, it makes sense
> to add locks to the sip_peer and sip_user structures. I'll go ahead and
> make these changes.
> Mark Michelson
Ah, I completely missed the fact that sip_peers are ao2 objects, and so they
already have a lock.
However, this makes things very tricky for implementing in 1.6.0. The reason is
that in that branch, we still have both sip_user and sip_peer structures. In
update_call_counter, we save a local pointers which may point to sip_user or
sip_peer counters. We can't do the same thing with their locks since they are
part of the ao2 data that we cannot directly access from within chan_sip. I
think we're stuck with the global lock solution for 1.6.0
More information about the asterisk-dev