[asterisk-dev] [Code Review]: adding shared locks around usage of odbc handle in res_odbc

[jrose]

> On Dec. 14, 2011, 11:08 a.m., Tilghman Lesher wrote:
> > /branches/1.8/res/res_odbc.c, line 731
> > <https://reviewboard.asterisk.org/r/1622/diff/1/?file=22212#file22212line731>
> >
> >     Actually, you expect "at least" a read lock.  There are cases below where you've obtained a write lock before calling the sanity checking function.
> >     
> >     Maybe create an internal version of this where the type of lock held is passed, and you escalate the lock only if it's necessary (and avoid a WARNING).
> 
> Tilghman Lesher wrote:
>     Also, because this is a public API, assuming that a lock is held isn't correct unless you document it in the header file.  That speaks doubly for a private version of this.

That warning message really has nothing to do with turning over the read lock to get the write lock.  I've created an alternate version of the function called ast_sanity_check_wrlocked which forgoes the lock switching step.

I'm taking over the patch for Walter since he can't finish it right now. It will be moving to a new review in a little while so that I can post the new diffs.


- jrose


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


On Dec. 14, 2011, 3:01 a.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1622/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2011, 3:01 a.m.)
> 
> 
> Review request for Asterisk Developers and Tilghman Lesher.
> 
> 
> Summary
> -------
> 
> See bug report: under certain circumstances res_odbc can be made to crash.
> 
> Adding a shared lock around usage of the con odbc handle is a way to fix this. When a reconnect is attempted, an exclusive lock is held. Result: no usage attempts while reconnecting.
> 
> This is hacked up quickly, so could use a bit of review; it wouldn't surprise me if I double-lock something somewhere. (And the assertion I added fails too.) Also, this could be the wrong way to approach this altogether.
> 
> @tilghman: is there a reason why ->up is set to 0 after query failure? (See same question in bug report at Cause (1).)
> 
> 
> This addresses bug ASTERISK-19011.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19011
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/include/asterisk/res_odbc.h 348047 
>   /branches/1.8/res/res_odbc.c 348047 
> 
> Diff: https://reviewboard.asterisk.org/r/1622/diff
> 
> 
> Testing
> -------
> 
> With the patch, I'm not getting the easy crashes anymore.
> 
> 
> Thanks,
> 
> wdoekes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120207/7d67622e/attachment.htm>