[asterisk-dev] [Code Review] Pimp My SIP: Further Threading Improvements

[Mark Michelson]

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

(Updated Feb. 25, 2013, 8:53 a.m.)


Review request for Asterisk Developers and Joshua Colp.


Changes
-------

Added sip_distributor.c


Summary
-------

The pool_shark branch was created to make the new SIP work make use of the threadpool. This set of changes improves upon the first set of changes by making things easier for people who write PJSIP modules. The changes are detailed below:

* The ast_sip_work structure and some of its methods have been removed in favor of the generic task serializer.

* A PJSIP module, the message distributor, has been added. This module has a high priority and gets called into after the PJSIP transport layer but before the PJSIP transaction layer. The distributor is used as a means of moving an incoming message from a PJSIP thread into a SIP servant thread. Then from the SIP servant thread, the message bubbles up through the PJSIP transaction layer, PJSIP dialog layer, and any relevant PJSIP modules that Asterisk has registered. This means that any PJSIP module that registers itself at the transaction layer or higher will have all of its incoming SIP messages already in a SIP servant thread when called back. Unless they need to serialize operations, they do not need to do any special processing to ensure the incoming message is handled in a specific thread. A good example of this is what the SIP OPTIONS handler does in this set of changes.

* Two new PJSIP modules have been added above the dialog layer. The endpoint module and the authenticator module are responsible for doing endpoint lookups and authentication for incoming requests. The placement of these modules means that they are only ever called into for out-of-dialog requests. This means that on an initial INVITE, or on a REGISTER, or on an OPTIONS request, or any other out-of-dialog request, the application-layer modules do not need to worry about endpoint lookup or authentication since that is done already for them. If they need the endpoint, they can call ast_pjsip_rdata_get_endpoint(). For dialog-forming application modules, they will want to save off the endpoint that the endpoint module finds since the endpoint will not be looked up again on in-dialog requests, meaning they will not be able to call ast_pjsip_rdata_get_endpoint().

* Modules that were passing incoming messages off to servants no longer need to do this, so they have been changed not to. So the session module and options module now handle incoming requests in-line rather than pushing them to a servant thread.

* Since it no longer makes sense to pre-allocate a serializer before an ast_sip_session, the functions for allocating SIP sessions has been changed not to take a serializer any more. Instead, the allocation routine creates its own. In addition, the behavior of endpoint references has changed from how it previously was. Now allocating a session results in the endpoint reference count increasing by one rather than having the session inherit the reference passed into it. This makes endpoint reference handling much cleaner, especially in the session module.

* To assist programmers who want to write PJSIP modules in Asterisk, there is a helper method called ast_sip_thread_is_servant() that has been added. It can be used to discern if the thread you currently are in is a SIP servant thread. This means it is not as necessary to dig into the PJSIP internals to know what type of thread you are in when called back. Instead, you can be safe and use this function. This function is currently unused though, since there hasn't been a need for it yet. This is because all uses of SIP servants where it may be questionable what type of thread we are in need to serialize the task anyway.

* Documentation of the SIP threadpool has been updated.


Diffs (updated)
-----

  /team/group/pimp_my_sip/channels/chan_gulp.c 381915 
  /team/group/pimp_my_sip/include/asterisk/res_sip.h 381915 
  /team/group/pimp_my_sip/include/asterisk/res_sip_session.h 381915 
  /team/group/pimp_my_sip/res/pjproject/pjsip/include/pjsip/sip_endpoint.h 381915 
  /team/group/pimp_my_sip/res/pjproject/pjsip/src/pjsip/sip_endpoint.c 381915 
  /team/group/pimp_my_sip/res/res_sip.c 381915 
  /team/group/pimp_my_sip/res/res_sip.exports.in 381915 
  /team/group/pimp_my_sip/res/res_sip/config_transport.c 381915 
  /team/group/pimp_my_sip/res/res_sip/sip_distributor.c PRE-CREATION 
  /team/group/pimp_my_sip/res/res_sip/sip_options.c 381915 
  /team/group/pimp_my_sip/res/res_sip_endpoint_identifier_ip.c 381915 
  /team/group/pimp_my_sip/res/res_sip_session.c 381915 

Diff: https://reviewboard.asterisk.org/r/2355/diff


Testing
-------

I have done some regression testing by running some calls in with SIPp and ensuring that the behavior is the same as before I started this work.


Thanks,

Mark

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