<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/3282/">https://reviewboard.asterisk.org/r/3282/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/3282/diff/1/?file=55021#file55021line1529" style="color: black; font-weight: bold; text-decoration: underline;">/branches/11/res/res_musiconhold.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static int local_ast_moh_start(struct ast_channel *chan, const char *mclass, const char *interpclass)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1529</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="n">state</span><span class="o">-></span><span class="n">class</span><span class="o">-></span><span class="n">realtime</span> <span class="o">&&</span> <span class="o">!</span><span class="n">ast_test_flag</span><span class="p">(</span><span class="n">global_flags</span><span class="p">,</span> <span class="n">MOH_CACHERTCLASSES</span><span class="p">)</span> <span class="o">&&</span> <span class="o">!</span><span class="n">strcasecmp</span><span class="p">(</span><span class="n">mohclass</span><span class="o">-></span><span class="n">name</span><span class="p">,</span> <span class="n">state</span><span class="o">-></span><span class="n">class</span><span class="o">-></span><span class="n">name</span><span class="p">))</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1523</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span><span class="k">if</span> <span class="p">(</span><span class="n">state</span><span class="o">-></span><span class="n">class</span><span class="o">-></span><span class="n">realtime</span> <span class="o">&&</span> <span class="o">!</span><span class="n">ast_test_flag</span><span class="p">(</span><span class="n">global_flags</span><span class="p">,</span> <span class="n">MOH_CACHERTCLASSES</span><span class="p">)</span> <span class="o">&&</span> <span class="o">!</span><span class="n">strcasecmp</span><span class="p">(</span><span class="n">mohclass</span><span class="o">-></span><span class="n">name</span><span class="p">,</span> <span class="n">state</span><span class="o">-></span><span class="n">class</span><span class="o">-></span><span class="n">name</span><span class="p">))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If you feel like it, you could also remove the !ast_test_flag(global_flags, MOH_CACHERTCLASSES) from here as well.</pre>
</div>
<br />
<p>- Matt Jordan</p>
<br />
<p>On March 2nd, 2014, 2:45 p.m. CST, Russell Bryant 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.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Russell Bryant.</div>
<p style="color: grey;"><i>Updated March 2, 2014, 2:45 p.m.</i></p>
<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;">I observed a crash in res_musiconhold on an Asterisk 11 system using realtime MOH. Investigation of the backtrace showed a corrupt mohclass, implying that it got destroyed before the code expected it to. I went looking for reference counting errors that could have caused this crash and this patch this result. It contains 2 changes.
1) Remove a usless block of code that was impossible to reach. There was even a comment indicating that it was impossible to reach. The conditional includes "!ast_test_flag(global_flags, MOH_CACHERTCLASSES)" and it's inside of an if block with the opposite check "ast_test_flag(global_flags, MOH_CACHERTCLASSES)". There's no good reason to keep it around.
2) A similar block to #1 contained a reference counting error. It stores state->class in the local variable mohclass without increasing its reference count. The reference count on mohclass is decremented at the end of the function. This block of code probably very rarely runs, which would help explain why this system was working fine for many months before experiencing a crash.</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/res/res_musiconhold.c <span style="color: grey">(409286)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3282/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>