[prev in list] [next in list] [prev in thread] [next in thread]
List: asterisk-dev
Subject: Re: [asterisk-dev] [Code Review] 2824: Fix DEBUG_THREADS when lock is acquired in __constructor__
From: "Mark Michelson" <reviewboard () asterisk ! org>
Date: 2013-09-05 22:02:19
Message-ID: 20130905220219.27458.61615 () hotblack ! digium ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2824/#review9614
-----------------------------------------------------------
Ship it!
/branches/1.8/main/utils.c
<https://reviewboard.asterisk.org/r/2824/#comment18789>
s/as/has/
- Mark Michelson
On Sept. 5, 2013, 9:39 p.m., David Lee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2824/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2013, 9:39 p.m.)
>
>
> Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
>
>
> Bugs: ASTERISK-22455
> https://issues.asterisk.org/jira/browse/ASTERISK-22455
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> This patch fixes some long-standing bugs in debug threads that were
> exacerbated with recent Optional API work in Asterisk 12.
>
> With debug threads enabled, on some systems, there's a lock ordering
> problem between our mutex and glibc's mutex protecting its module list
> (Ubuntu Lucid, glibc 2.11.1 in this instance). In one thread, the module
> list will be locked before acquiring our mutex. In another thread, our
> mutex will be locked before locking the module list (which happens in
> the depths of calling backtrace()).
>
> This patch fixes this issue by moving backtrace() calls outside of
> critical sections that have the mutex acquired. The bigger change was to
> reentrancy tracking for ast_cond_{timed,}wait, which wrongly assumed
> that waiting on the mutex was equivalent to a single unlock (it actually
> suspends all recursive locks on the mutex).
>
>
> Diffs
> -----
>
> /branches/1.8/main/utils.c 398497
> /branches/1.8/main/lock.c 398497
> /branches/1.8/include/asterisk/lock.h 398497
>
> Diff: https://reviewboard.asterisk.org/r/2824/diff/
>
>
> Testing
> -------
>
> Asterisk 12, DEBUG_THREADS+OPTIONAL_API on Ubuntu Lucid starts without
> deadlock.
>
> Asterisk 1.8, DEBUG_THREADS runs fine.
>
>
> Thanks,
>
> David Lee
>
>
[Attachment #5 (text/html)]
<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/2824/">https://reviewboard.asterisk.org/r/2824/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<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/2824/diff/2/?file=45662#file45662line588" \
style="color: black; font-weight: bold; text-decoration: \
underline;">/branches/1.8/main/utils.c</a> <span style="font-weight: normal;">
(Diff revision 2)
</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; ">struct \
thr_lock_info {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">588</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="tb"> </span><span class="tb"> </span><span class="cm">/*! A condition as \
suspended this lock */</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;">s/as/has/</pre> </div>
<br />
<p>- Mark</p>
<br />
<p>On September 5th, 2013, 9:39 p.m. UTC, David Lee 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, Matt Jordan and Mark Michelson.</div>
<div>By David Lee.</div>
<p style="color: grey;"><i>Updated Sept. 5, 2013, 9:39 p.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-22455">ASTERISK-22455</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;">This patch fixes some long-standing bugs in debug threads that were \
exacerbated with recent Optional API work in Asterisk 12.
With debug threads enabled, on some systems, there's a lock ordering
problem between our mutex and glibc's mutex protecting its module list
(Ubuntu Lucid, glibc 2.11.1 in this instance). In one thread, the module
list will be locked before acquiring our mutex. In another thread, our
mutex will be locked before locking the module list (which happens in
the depths of calling backtrace()).
This patch fixes this issue by moving backtrace() calls outside of
critical sections that have the mutex acquired. The bigger change was to
reentrancy tracking for ast_cond_{timed,}wait, which wrongly assumed
that waiting on the mutex was equivalent to a single unlock (it actually
suspends all recursive locks on the mutex).</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;">Asterisk 12, DEBUG_THREADS+OPTIONAL_API on Ubuntu Lucid starts without \
deadlock.
Asterisk 1.8, DEBUG_THREADS runs fine.</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/1.8/main/utils.c <span style="color: grey">(398497)</span></li>
<li>/branches/1.8/main/lock.c <span style="color: grey">(398497)</span></li>
<li>/branches/1.8/include/asterisk/lock.h <span style="color: \
grey">(398497)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2824/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic