<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/1177/">https://reviewboard.asterisk.org/r/1177/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This patch assumes that every time a pvt lock is held, the pvt->owner lock must be held as well. While this is true for many instances, this is not always true.</pre>
<br />
<p>- David</p>
<br />
<p>On April 13th, 2011, 9:39 a.m., astmiv wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By astmiv.</div>
<p style="color: grey;"><i>Updated 2011-04-13 09:39:20</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/view.php?id=19108">19108</a>,
<a href="https://issues.asterisk.org/view.php?id=19112">19112</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/channels/chan_sip.c <span style="color: grey">(313525)</span></li>
<li>/branches/1.8/channels/sip/dialplan_functions.c <span style="color: grey">(313525)</span></li>
<li>/branches/1.8/channels/sip/include/sip.h <span style="color: grey">(313525)</span></li>
<li>/branches/1.8/channels/sip/include/sip_utils.h <span style="color: grey">(313525)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1177/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>