[reSIProcate] memory management and asynchronous activity in TransactionState.cxx

[Scott Godin]

...some comments inline...


On Thu, Nov 14, 2013 at 1:49 PM, Daniel Pocock <daniel at pocock.com.au> wrote:

>
>
>
>
> I've been looking over TransactionState.cxx with the intention of adding
> more asynchronous possibilities, for example, allowing MessageDecorator
> to do some async jobs
>
> A few things stand out:
>
> - One problem area is memory management, making sure that the SipMessage
> is not inadvertently deleted too soon (while async in progress) and not
> left hanging around either.  I notice that the SipMessage
> (mNextTransmission) is just a regular pointer - is there any reason not
> to use SharedPtr or auto_ptr for this?
>

[Scott]  The original stack developers intentionally kept SharedPtr's out
of the core stack for performance reasons.  Each SharedPtr access requires
a mutex lock.  Not sure if the same argument applies to auto_ptr.  There
was a commit years ago by a company that was contributing to resip to
convert all SipMessage storage to use SharedPtr's.  This checkin was
reverted, due to the reasons I listed.


> - the last async activity that takes place before decoration is the DNS
> activity (mDnsResult) so I thought that might provide a useful model for
> other async activities.  Should this be further generalised, e.g.
> instead of having a bool mWaitingForDnsResult, keeping some enum that
> represents the state?  Or is it better to keep extra flags, e.g. bool
> mWaitingForDecorator?
>

[Scott]  Either way is fine with me.


> - are there any other asynchronous activities that anybody can envisage
> adding to this part of the code in future?
>
> - to avoid disruption to existing MessageDecorator users, I'm likely to
> add some virtual method with a default implementation that calls the
> existing synchronous decorateMessage(), e.g.
>
>       virtual bool decorateMessage(SipMessage &msg,
>                                   const Tuple &source,
>                                   const Tuple &destination,
>                                   const Data& sigcompId,
>                                   Fifo<TransactionMessage>&
> asyncHandlerFifo)
>       {
>          decorateMessage(msg, source, destination, sigcompId);
>          return true;
>       }
>
> and anybody who wants to do something more lengthy can override that
> method, start a thread, return false and post the result back on the
> fifo when ready.  Does this seem like a reasonable way to avoid API
> disruption?
>

[Scott]  it would be better if existing applications didn't have any
performance penalties introduced as a result of adding async support.


>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel at resiprocate.org
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20131114/ed4c38da/attachment.htm>