[asterisk-dev] [Code Review] 2826: Debug threads: avoid double-initialization of lock tracking

[rmudgett]

> On Sept. 19, 2013, 10:06 p.m., rmudgett wrote:
> > /branches/1.8/main/lock.c, lines 73-90
> > <https://reviewboard.asterisk.org/r/2826/diff/3/?file=45654#file45654line73>
> >
> >     You could reduce the pain of the global lock here by:
> >     if *plt
> >       return *plt
> >     lock global reentrancy lock
> >     if *plt
> >       unlock global reentrancy lock
> >       return *plt
> >     
> >     get memory for plt and initialize it
> >     
> >     *plt = initialized
> >     unlock global reentrancy lock
> >     return *plt
> 
> David Lee wrote:
>     The optimizer throws a wrench into that. Hence my comment about a double-checked lock.
> 
> rmudgett wrote:
>     Instead of checking if *plt is not NULL and returning it since setting a pointer cannot always be done atomically.  Use a trace initialized flag that can be set atomically before returning *plt and do the double-checked lock.
> 
> David Lee wrote:
>     Optimizer still has wrench-throwing capabilities. Even if the flag can
>     be set atomically, the compiler is allowed to reorder instructions, so
>     it could be set before the pointer update.
> 
> rmudgett wrote:
>     The optimizer cannot violate code sequence points without breaking the language.  How the function is coded will determine what the optimizer can do.

Instead of using the pointer as a flag variable which cannot be changed atomically, use a volatile int as the flag variable.  If you still have concernes then you can use ast_atomic_fetchadd_int() to examine and change the int flag.


- rmudgett


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2826/#review9741
-----------------------------------------------------------


On Sept. 5, 2013, 6:24 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2826/
> -----------------------------------------------------------
> 
> (Updated Sept. 5, 2013, 6:24 p.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> Bugs: ASTERISK-19463
>     https://issues.asterisk.org/jira/browse/ASTERISK-19463
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch corrects a consistency issue with debug threads that I
> noticed while fixing ASTERISK-22455.
> 
> The initialization of a mutex's lock tracking structure was not
> protected in a critical section. This is fine for any mutex that is
> explicitly initialized, but a static mutex may have its lock tracking
> double-initialized if multiple threads attempt the first lock
> simultaneously.
> 
> This patch creates a global mutex to properly serialize the
> initialization of the lock tracking structure for a mutex. It also
> changes lock.c to properly handle allocation failures of the lock
> tracking structure.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/main/lock.c 398421 
>   /branches/1.8/include/asterisk/lock.h 398421 
> 
> Diff: https://reviewboard.asterisk.org/r/2826/diff/
> 
> 
> Testing
> -------
> 
> I propose we setup Bamboo to run the TestSuite with DEBUG_THREADS
> enabled on this branch nightly for a few weeks.
> 
> 
> Thanks,
> 
> David Lee
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131125/ce010488/attachment.html>