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

[Scott Godin <sgodin@xxxxxxxxxxxxxxx>]

...some comments inline...

On Thu, Nov 14, 2013 at 1:49 PM, Daniel Pocock <daniel@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel