<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/2594/">https://reviewboard.asterisk.org/r/2594/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Looks good - thanks for putting the refactoring on a separate review</pre>
<br />
<p>- Matt</p>
<br />
<p>On June 8th, 2013, 6:57 a.m. UTC, Alec Davis wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/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 Alec Davis.</div>
<p style="color: grey;"><i>Updated June 8, 2013, 6:57 a.m.</i></p>
<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/jira/browse/ASTERISK-21409">ASTERISK-21409</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<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;">1). When touching the bridgecallno, we need to lock it.
2). stop_stuff() which calls iax2_destroy_helper()
Assumes the lock on the pvt is already held, when iax2_destroy_helper() is called.
Thus we need to lock the bridgecallno pvt before we call stop_stuff(iaxs[fr->callno]->bridgecallno);
3). When evaluating the state of 'callno->transferring' of the current leg, we can't change it to READY unless the bridgecallno is locked.
Why, if we are interrupted by the other call leg before 'transferring = TRANSFER_RELEASED', the interrupt will find that it is READY and that the bridgecallno is also READY so Releases the legs.
Then the first call leg in this example, finishes execution, and Releases the legs AGAIN!!!!!!!
Interleaving thread execution gets interesting as well - see timeline below from June 5.
Debug captures when it went wrong.
[May 31 14:44:01] VERBOSE[30820] chan_iax2.c: -- Channel 'IAX2/auckland-13262' ready to transfer
[May 31 14:44:01] VERBOSE[30824][C-00000536] chan_iax2.c: -- Channel 'IAX2/auckland-20457' ready to transfer
[May 31 14:44:01] VERBOSE[30824][C-00000536] chan_iax2.c: -- Releasing IAX2/auckland-20457 and IAX2/auckland-13262
[May 31 14:44:01] VERBOSE[30820] chan_iax2.c: -- Releasing IAX2/auckland-13262 and IAX2/auckland-20457
[May 31 14:44:01] DEBUG[30824][C-00000536] sched.c: Attempted to delete nonexistent schedule entry 209951!
[May 31 14:44:01] ERROR[30824][C-00000536] lock.c: chan_iax2.c line 1918 (iax2_destroy_helper): mutex '&iaxsl[pvt->callno]' freed more times than we've locked!
[May 31 14:44:01] ERROR[30824][C-00000536] lock.c: chan_iax2.c line 1918 (iax2_destroy_helper): Error releasing mutex: Operation not permitted
[Jun 5 19:53:43] VERBOSE[25606][C-00000000] chan_iax2.c: -- Channel 'IAX2/auckland-19065' ready to transfer
[Jun 5 19:53:43] VERBOSE[25604][C-00000000] chan_iax2.c: -- Channel 'IAX2/auckland-20047' ready to transfer
[Jun 5 19:53:43] VERBOSE[25606][C-00000000] chan_iax2.c: -- Releasing IAX2/auckland-19065 and IAX2/auckland-20047
[Jun 5 19:53:43] VERBOSE[25604][C-00000000] chan_iax2.c: -- Releasing IAX2/auckland-20047 and IAX2/auckland-19065
[Jun 5 19:53:43] VERBOSE[25606][C-00000000] chan_iax2.c: -- Channel 'IAX2/auckland-19065' finished transfer
[Jun 5 19:53:43] DEBUG[25604][C-00000000] sched.c: Attempted to delete nonexistent schedule entry 17!
[Jun 5 19:53:43] VERBOSE[25604][C-00000000] chan_iax2.c: -- Channel 'IAX2/auckland-20047' finished transfer
A time line of 2 threads interleaving that shows how the Jun 5 capture may have come about.
The execution path seems to switch threads as we print/log data.
thread 25606 thread 25604
====================== =======================
if US == BEGIN
US = READY
"ready to transfer"
if US == BEGIN
US = READY
"ready to transfer"
if THEY == READY
"Releasing .."
if THEY == READY
"Releasing .."
THEY = RELEASED
US = RELEASED
stopstuff(US)
stopstuff(THEM)
"finished transfer"
THEY = RELEASED
US = RELEASED
stopstuff(US)
stopstuff(THEM) (sched.c: Attempted to delete nonexistent schedule entry 17!)
"finished transfer"
</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;">Yes. Between sites. many test calls and real calls being transferred back down the same trunk</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>branches/11/channels/chan_iax2.c <span style="color: grey">(385594)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2594/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>