[reSIProcate] MessageDecorator bug?
Byron Campen
bcampen at estacado.net
Wed Oct 29 22:22:38 CDT 2008
The reason we decided to move toward
SipMessage::addOutboundDecorator() taking ownership is because
MessageDecorator is now required to carry enough state to roll-back
any changes it has made to the SipMessage; it makes sense for each
SipMessage (or copy thereof) to have its own copy of the decorator. If
you need a single place to keep state, you can use a subclass of
MessageDecorator that has a reference to that single place.
In any case, from what I can tell looking at the code, we were adding
multiple copies of the decorator in DUM before the behavior was
changed. The point is that DialogUsageManager::send() takes a
SharedPtr<SipMessage> that is being held on to by some subclass of
BaseUsage (for example, ClientPublication::mPublish), putting a
decorator on it, and passing it down to the stack (which then clones
the SipMessage). Meanwhile, many of the subclasses of BaseUsage hold
onto that same SharedPtr<SipMessage>, with the decorator still
attached, and just fiddle the CSeq and various other things, and later
call DialogUsageManager::send() with the same message with the
decorator already on it. DUM adds another copy, rinse, lather, repeat.
What I think we need to do is write
DialogUsageManager::send(auto_ptr<SipMessage>), and have
DialogUsageManager::send(SharedPtr<SipMessage>) just turn around and
clone the message into an auto_ptr, and call the auto_ptr version of
the function. Also, having this function take an auto_ptr would mean
that DUM could use SipStack::send(auto_ptr<SipMessage>), which would
actually allow us to save cycles in many cases.
Best regards,
Byron Campen
> I am using outbound decorators myself, but I haven't
> tried release 1.4.
> I don't understand why the cloning was introduced. I
> have a single outbound decorator which I'm holding a
> reference to. I guess that's the reason why it is a
> shared_ptr. When the STUN addresses change I know
> that I have this instance only where I need to change
> the public ip address. Not multiple instances that
> constantly clone themselves. I would vote to revert to
> the original behaviour. It works, it's tested and it's
> proven not to leak.
>
>
> Best regards,
> Freundliche Grüße,
>
> Matthias Moetje
> ______________________________________________
>
> TERASENS GmbH Phone: +49.89.143370-0
> Augustenstraße 24 Fax: +49.89.143370-22
> 80333 Munich e-mail: info at terasens.com
> GERMANY Web: www.terasens.com
> ______________________________________________
>
>
>
>> -----Original Message-----
>> From: resiprocate-devel-bounces at resiprocate.org [mailto:resiprocate-devel-
>> bounces at resiprocate.org] On Behalf Of Byron Campen
>> Sent: Mittwoch, 29. Oktober 2008 21:54
>> To: Daniel Shi
>> Cc: resiprocate-devel; Jason Fischl; Derek MacDonald
>> Subject: Re: [reSIProcate] MessageDecorator bug?
>>
>> Looking at the old code, it looks like daniel set this stuff
>> up with
>> SharedPtr a few years ago. Is there a reason we need to keep doing
>> it this
>> way?
>>
>> Best regards,
>> Byron Campen
>>
>>> Ugh. Yeah, this is completely broken.
>>>
>>> The reason this is biting us is that DialogUsageManager is
>>> adding the outbound decorators to long-lifetime
>>> SharedPtr<SipMessage>
>>> that live in the various subclasses of DialogUsage. We either
>>> need to
>>> have the subclasses set up the outbound decorators themselves, or
>>> stop
>>> passing their outgoing traffic with a SharedPtr (ie, have
>>> DialogUsage
>>> clone the SipMessage, put it in an auto_ptr, and fling _that_ at
>>> DialogUsageManager). My vote is for the latter, since using
>>> SharedPtr
>>> in this case doesn't make much sense anyway. We should be passing
>>> auto_ptr instead.
>>>
>>> Best regards,
>>> Byron Campen
>>>
>>>> Hi,
>>>> I'm testing DUM using Resiprocate release 1.4 (SVN head) and have
>>>> noticed a strange behaviour about MessageDecorator invocation while
>>>> sending a SIP message.
>>>>
>>>> To reproduce the issue I changed the sample application
>>>> basicRregister.cxx file (in "dum/test/directory") with some
>>>> modifications: I added my class SipMessageDecorator, invoked
>>>> setOutboundDecorator(), added logs and removed unregistration so
>>>> that
>>>> the REGISTER message is always sent.
>>>>
>>>> I set my MessageDecorator class in the main with
>>>> setOutboundDecorator() method, in this way:
>>>> SharedPtr<SipMessageDecorator> messageDecorator(new
>>>> SipMessageDecorator());
>>>> clientDum.getMasterProfile()-
>>>> >setOutboundDecorator(messageDecorator);
>>>>
>>>> After sending the first REGISTER message, the method
>>>> decorateMessage()
>>>> in MessageDecorator class is called once, but the second time it is
>>>> called consecutively twice with the same message, at third REGISTER
>>>> three times (always same message) and so on.
>>>>
>>>> I noticed that in DialogUsageManager the send() method adds a
>>>> MessageDecorator clone to mOutboundDecorators and the
>>>> SipMessage::callOutboundDecorator() method calls decorateMessage()
>>>> for each item in vector mOutboundDecorators, but this vector always
>>>> grows.
>>>>
>>>> I attached to this mail the changed sample application that I
>>>> tested
>>>> using repro and the produced log file.
>>>>
>>>> Is there a bug or my mistake?
>>>>
>>>> Thank you in advance.
>>>> Best regards
>>>>
>>>> Andrea Chiappori
>>>> <
>>>> basicRegister
>>>> .cxx><test.log>_______________________________________________
>>>> resiprocate-devel mailing list
>>>> resiprocate-devel at resiprocate.org
>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>
>>> _______________________________________________
>>> 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/20081029/3886a2b9/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2482 bytes
Desc: not available
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20081029/3886a2b9/attachment.bin>
More information about the resiprocate-devel
mailing list