[OpenSIPS-Devel] [ opensips-Patches-3042432 ] [pua] add a "publish" queue to avoid deadlock

[Anca Vamanu]

Hi Alex,

On 08/16/2010 03:38 PM, Alex Hermann wrote:
> Some more issues:
>
> Failure handling in callback. After an error "return" isn't appropriate if
> another publish is waiting to be sent.
>
> Also in the callback, handling is a bit inefficient: update_htable is called,
> and directly after exactly the same logic as in the beginning of that function
> is done again (locking the hash and finding the entry). I think it is cleaner
> to take the lock once and keep it as long as necessary.
>    
You might be right here - since the operations that are done outside the 
lock are not that time consuming.

> In construct_pending_publ, extra_headers is taken from presentity. It should
> instead come from the stored publ_t (note that the extra_headers aren't copied
> either from the original publ_info_t). This bug causes a segfault.
>
>    
I choose to not store the extra_headers and use the ones in the 
presentity structure as I wanted the stored publish request to be small 
and considered that since it is about the same source that request for a 
new Publish to be sent, there are very small chances if any that new 
extra_headers will be required.
And what about the crash? There is the bug here that leads to a crash?
> Why isn't the publ_info_t supplied to send_publish stored in its entirety? Now
> a function build_pending_publ is called and later on construct_pending_publ to
> handle it again. Can't these be combined to store (a copy of) the original
> publ_info_t?
>
>    
Sure I could have used the same structure. I chose to define a new one 
because only 4 fileds are needed and the original publ_info_t structure 
has 11 fields.


Regards,

-- 
Anca Vamanu
www.voice-system.ro


> I'm willing to help coding a clean and working pua implementation, if
> interested in working together, please contact me.
>