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

[Tilghman Lesher]

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



/branches/1.8/res/res_odbc.c
<https://reviewboard.asterisk.org/r/1622/#comment9261>

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



/branches/1.8/res/res_odbc.c
<https://reviewboard.asterisk.org/r/1622/#comment9256>

    This one should not be necessary, because a transactional handle by definition isn't shared.



/branches/1.8/res/res_odbc.c
<https://reviewboard.asterisk.org/r/1622/#comment9257>

    Ditto.



/branches/1.8/res/res_odbc.c
<https://reviewboard.asterisk.org/r/1622/#comment9258>

    Ditto.



/branches/1.8/res/res_odbc.c
<https://reviewboard.asterisk.org/r/1622/#comment9259>

    A note would be appreciated here that this may only be called for _shared_ connections, because otherwise, the lack of setting ->used breaks EOR_TX searching.


- Tilghman


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/dfc20e7e/attachment.htm>