Byron,
thanks for the clarification. I never use SipMessage::addOutoundDecorator()
but rather
Profile::setOutboundDecorator(SharedPtr<MessageDecorator>
outboundDecorator) and let dum add the decorator itself.
I guess this interface wouldn’t need to be changed as dum could
create an auto_ptr-clone from this instance?
Matthias
From: Byron Campen
[mailto:bcampen@xxxxxxxxxxxx]
Sent: Donnerstag, 30. Oktober 2008 04:23
To: Matthias Moetje
Cc: 'Daniel Shi'; 'resiprocate-devel'; 'Jason Fischl'; 'Derek MacDonald'
Subject: Re: [reSIProcate] MessageDecorator bug?
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.
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@xxxxxxxxxxxx
GERMANY
Web:
www.terasens.com
______________________________________________
-----Original Message-----
From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx [mailto:resiprocate-devel-
bounces@xxxxxxxxxxxxxxx]
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@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
|