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

[Tilghman Lesher]

> 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).

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.


- Tilghman


-----------------------------------------------------------
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/20111214/370e3e36/attachment.htm>