[asterisk-dev] [Code Review] Make sure that the lock order for sip_pvt and sip_pvt->owner is allways right. This to prevent deadlocks.

[astmiv]

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

Review request for Asterisk Developers.


Summary
-------

The order in which sip_pvt and channels are locked is important.
For different reported dealock bugs it appears this locking order is not respected.
After examing different "core show lock" files it appears that the deadlock always happens when one thread first locks the sip_pvt->owner channel and later tries a lock for the sip_pvt itself. Another thread first locks the sip_pvt and then tries to lock the sip_pvt->owner channel (this is wrong).

We could try to fix this wrong lock order but might be very complicated because they happen in two different threads.

This update/patch makes sure that when a sip_pvt is locked it's sip_pvt->owner is always locked before.

sip_pvt lock: Check if sip_pvt has owner. If so wait for lock on sip_pvt->owner and remember that the owner was locked by the sip_pvt lock. When owner is locked lock sip_pvt itself.
sip_pvt unlock: Unlock the sip_pvt. If we have an owner and the sip_pvt_lock locked it unlock owner.
sip_pvt trylock: Check if sip_pvt has owner. If so wait for lock on sip_pvt->owner and remember that the owner was locked by the sip_pvt lock. When owner is locked do a trylock for sip_pvt. If the trylock is unsuccessful unlock owner.

For this to work all right we also have to monitor the change of the sip_pvt->owner. I created a separate function for this and altered the code where necessary.

The diff is not clean yet and is only a working concept. 


This addresses bugs 19108 and 19112.
    https://issues.asterisk.org/view.php?id=19108
    https://issues.asterisk.org/view.php?id=19112


Diffs
-----

  /branches/1.8/channels/chan_sip.c 313525 
  /branches/1.8/channels/sip/dialplan_functions.c 313525 
  /branches/1.8/channels/sip/include/sip.h 313525 
  /branches/1.8/channels/sip/include/sip_utils.h 313525 

Diff: https://reviewboard.asterisk.org/r/1177/diff


Testing
-------

I have it currently running on a test system, 1.8.3.2, for which I was able to reproduce issue 19108. With this patch the reported deadlock of issue 19108 is history.
I had to increase AST_MAX_LOCKS from 64 to 300 because one thread could not set any locks because it reached the limit.
Currently still testing with sipp


Thanks,

astmiv

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110413/35f92267/attachment.htm>