[asterisk-dev] Concurrency threading problem in chan_sip

[Russell Bryant]

Nick Gorham wrote:
> My reading of whats going on is that the scheduler can call
> 
>     static int retrans_pkt(const void *data)
> 
> And this can run as far as the
> 
>     /* Lock channel PVT */
>     sip_pvt_lock(pkt->owner);
> 
> Then it sits there waiting for the lock to be released. The lock I 
> suspect was put on this in
> 
>     static void __sip_ack(struct sip_pvt *p, int seqno, int resp, int 
> sipmethod)
> 
> which runs its course, and eventually calls
> 
>     UNLINK(cur, p->packets, prev);
>     dialog_unref(cur->owner);
>     ast_free(cur);
>     break;
> 
> The ast_free releases the memory, but its still referenced by the pkt in 
> the thread in retransmit_pkt. Then it wakes up the locked thread by caling
> 
>     sip_pvt_unlock(p);
> 
> But now when the call in retransmit_pkt continues, the memory its using 
> has actually been free'd by now, so it leads open the chance of 
> corrupting the heap with the expected entertaining problems.
> 
> Hopefully there is enough to go on with the above, I am unsure just how 
> to fix this (otherwise I would have tried)

I believe that your analysis is probably correct, though I have not gone and
read the code myself.  I believe Tilghman has written a patch to ensure that SIP
packets are reference counted to resolve this exact type of situation.
Hopefully he can speak up and point you to the issue that has the patch for
testing ...

-- 
Russell Bryant
Senior Software Engineer
Open Source Team Lead
Digium, Inc.