<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/1708/">https://reviewboard.asterisk.org/r/1708/</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;">I think that because of your change to agent_hangup(), there's a new potential deadlock. Allow me to explain:
The competing threads are the agent's channel thread and the thread that processes, say, the AMI agents action.
First, the channel thread issues a hangup, so agent_hangup is called with the channel locked.
Meanwhile, someone issues the Agents action and grabs the list lock for the agents.
The hangup now wants to grab the list lock, but cannot until the AMI action is completed.
So now, the AMI thread does its business where it grabs the pvt->owner, unlocks, and then attempts to grab the channel lock.
We now have a deadlock because the AMI thread has the list lock and is trying to get the channel lock, and the channel thread has the channel lock and is trying to get the list lock.</pre>
<br />
<p>- Mark</p>
<br />
<p>On February 1st, 2012, 2:20 p.m., jrose wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/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, Mark Michelson, rmudgett, and Matt Jordan.</div>
<div>By jrose.</div>
<p style="color: grey;"><i>Updated Feb. 1, 2012, 2:20 p.m.</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;">Also adds locking to a number of other functions which are calling ast_bridged_channel (which is documented as requiring a lock for safe running, which was the purpose of irroot's original locking patch in action_agents.
Unlike the other patch on reviewboard right now, this opts to enforce locking order instead of using deadlock avoidance.
Celebrity endorsement: "This looks incredibly sane to me." - MMichelson</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;">Not much I'm afraid to say. I can't reproduce the issue since it involves real world use over a period of time. I'll ask Alex Villacís Lasso to give it a shot though.</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/jira/browse/ASTERISK-19285">ASTERISK-19285</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_agent.c <span style="color: grey">(353685)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1708/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>