[asterisk-dev] [Code Review] Make new SIP work make use of threadpool

[jcolp]

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



/team/group/pimp_my_sip/channels/chan_gulp.c
<https://reviewboard.asterisk.org/r/2305/#comment14866>

    Minor tweak - improve the log message.



/team/group/pimp_my_sip/channels/chan_gulp.c
<https://reviewboard.asterisk.org/r/2305/#comment14867>

    This should be synchronous. When this function returns the core expects the channel driver to no longer have a reference to the channel and also expects the pvt to be NULL. If it's not it'll try to do "stuff" to free it. I think we also need to do an audit of stuff that may be pushed after this is pushed. Media updates, remote session termination, etc. They may need special checks to ensure stuff is still there (ie: the channel).



/team/group/pimp_my_sip/res/res_sip.c
<https://reviewboard.asterisk.org/r/2305/#comment14868>

    And by awful things you mean certain death with an assertion, lawl.



/team/group/pimp_my_sip/res/res_sip.c
<https://reviewboard.asterisk.org/r/2305/#comment14869>

    I improved module reference count stuff so this may actually work now.



/team/group/pimp_my_sip/res/res_sip_session.c
<https://reviewboard.asterisk.org/r/2305/#comment14870>

    We should document the limitations of this in case people try to do stuff that won't work (ala mod_data).


- jcolp


On Feb. 6, 2013, 5:13 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2305/
> -----------------------------------------------------------
> 
> (Updated Feb. 6, 2013, 5:13 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and jcolp.
> 
> 
> Summary
> -------
> 
> This changeset introduces threadpool usage into the new SIP work. Function calls that originate from the Asterisk core (such as channel callbacks) and from PJSIP's event handling thread (such as incoming request/response callbacks) now push their work into the SIP threadpool.
> 
> The benefits to this changeset are:
> 1) Frees up PJSIP event handling threads so that they can process more incoming SIP messages at a time
> 2) Places similar SIP tasks into their own task queue so that they can be completed in sequence, thus limiting resource contention. So far, session tasks are the only ones that use this mechanism
> 
> I added a new API call called ast_sip_push_task_synchronous(), that allows for you to push a task to a servant and block until the task completes. This is used in several cases where Asterisk threads need to make use of a PJSIP feature but need to have the result of the pushed task before returning. A good example of this is gulp_request() in chan_gulp. We need to call some PJSIP routines to set up a UAC and a dialog and such. But we also need to return a new ast_channel before gulp_request() returns.
> 
> Please have a look over this to see if I've made any obvious mistakes (e.g. memory leaks, tasks being not being executed in the threadpool when they should be)
> 
> 
> Diffs
> -----
> 
>   /team/group/pimp_my_sip/channels/chan_gulp.c 381009 
>   /team/group/pimp_my_sip/include/asterisk/res_sip.h 381009 
>   /team/group/pimp_my_sip/include/asterisk/res_sip_session.h 381009 
>   /team/group/pimp_my_sip/include/asterisk/threadpool.h 381009 
>   /team/group/pimp_my_sip/main/taskprocessor.c 381009 
>   /team/group/pimp_my_sip/main/threadpool.c 381009 
>   /team/group/pimp_my_sip/res/res_sip.c 381009 
>   /team/group/pimp_my_sip/res/res_sip.exports.in 381009 
>   /team/group/pimp_my_sip/res/res_sip/config_transport.c 381009 
>   /team/group/pimp_my_sip/res/res_sip/sip_options.c 381009 
>   /team/group/pimp_my_sip/res/res_sip_session.c 381009 
> 
> Diff: https://reviewboard.asterisk.org/r/2305/diff
> 
> 
> Testing
> -------
> 
> I've run the OPTIONS test in the testsuite and can confirm that Asterisk shutdown does not crash Asterisk any more. I also ran incoming and outgoing calls and ensured that they completed correctly with no issue.
> 
> 
> Thanks,
> 
> Mark
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130211/74de9593/attachment-0001.htm>