[asterisk-dev] [Code Review]: Lock the peer->mwipvt to avoid crashed in sip history management when overflowing history entries

[Olle E Johansson]

> On Aug. 18, 2011, 10:03 a.m., David Vossel wrote:
> > ./trunk/channels/chan_sip.c, lines 25659-25671
> > <https://reviewboard.asterisk.org/r/1373/diff/2/?file=18508#file18508line25659>
> >
> >     What is the established locking order between peers and sip_pvts?  The peer is locked during this function.

That's a very good question. I don't see us touching any peer between the locks, so I think it's not important here. Anyone else that have an answer or an opinion?


- Olle E


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


On Aug. 18, 2011, 7:15 a.m., Olle E Johansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1373/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2011, 7:15 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Historically, mwi was sent by one thread (monitor) and did not need any locking. With the addition of event driven MWI, we now have two threads, the event thread and the monitor thread for retransmits, that work on the same pvt. We have had multiple crashes in 1.6.0 caused by fights over the sip history linked list and memory corruption when removing entries (when the list overflows). Looking at it, we realized that no locking was done so the two threads where allowed to have fun with the same pvt at the same time.
> 
> This affects everything since the historic day we released 1.6.0
> 
> 
> This addresses bug asterisk-18288.
>     https://issues.asterisk.org/jira/browse/asterisk-18288
> 
> 
> Diffs
> -----
> 
>   ./trunk/channels/chan_sip.c 332444 
> 
> Diff: https://reviewboard.asterisk.org/r/1373/diff
> 
> 
> Testing
> -------
> 
> Compiled fine. Felt fine. Under testing now.
> 
> 
> Thanks,
> 
> Olle E
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110818/2a09da7f/attachment.htm>