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

[Byron Campen <docfaraday@xxxxxxxxx>]

On 11/14/13 11:47 AM, Scott Godin wrote:

...

On Thu, Nov 14, 2013 at 2:26 PM, Byron Campen <docfaraday@xxxxxxxxx> wrote:

On 11/14/13 11:15 AM, Scott Godin wrote:

...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.

 

     Are we still using that terrible SharedPtr implementation? Newer versions of that are _much_ more efficient. As for auto_ptr, it is extremely efficient. As for that commit years ago, that was violently rejected due to the threadsafety nastiness it invited.

[Scott] I lifted the SharedPtr implementation from boost.  Are you saying that boost had a bad implementation that has been improved since?  Do you support the use of SharedPtr's or auto pointers in the stack then?  I would defer to you, since you did so much of the stack optimization work in the past.  : )

     Yeah, the boost sharedptrs have gotten waaaaaay better; they just use low-level lock-free atomic stuff now (I assume the c++11 shared_ptr stuff is basically identical). If we want to make more use of these, we may want to consider updating our copy, use it in place of c++11 shared_ptr when that isn't available (with the same name), and maybe provide the old SharedPtr alias to keep source compat for downstream. I'm not quite sure what this would mean for ABI compat though. However, so much of resip assumes strong ownership that I don't think that sharing anything across, say, the stack/TU boundary is going to be a good idea. auto_ptr I'm totally fine with.

Best regards,
Byron Campen