| < Previous by Date | Date Index | Next by Date > |
| < Previous in Thread | Thread Index | Next in Thread > |
|
Never heard from anybody if this patch was correct… -Aron From:
resiprocate-devel-bounces@xxxxxxxxxxxxxxx
[mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of Aron
Rosenberg This is an updated patch which removes the use mRemoteTarget
completely for the new subscription and drops the appDialogSet reuse Its untested right now, but wanted to see if it was conceptually
correct. Index: dum/ClientSubscription.cxx =================================================================== --- dum/ClientSubscription.cxx (revision 8133) +++
dum/ClientSubscription.cxx
(working copy) @@ -142,9 +142,9 @@
msg.exists(h_Expires) && msg.header(h_Expires).value() > 0) { InfoLog (<<
"Received 481 to SUBSCRIBE, reSUBSCRIBEing (presence server probably
restarted) " -
<< mDialog.mRemoteTarget); +
<< mLastRequest->header(h_To)); - SharedPtr<SipMessage> sub
= mDum.makeSubscription(mDialog.mRemoteTarget, getEventType(),
getAppDialogSet()->reuse()); + SharedPtr<SipMessage> sub
= mDum.makeSubscription(mLastRequest->header(h_To), getEventType()); mDum.send(sub); delete this; @@ -188,17 +188,10 @@
DebugLog(<< "Application requested immediate retry on
Retry-After"); //!dcm!
-- why isn't this just a refresh--retry after might be //middle
element and not indicate dialog destruction
- if
(mDialog.mRemoteTarget.uri().host().empty()) - { -
SharedPtr<SipMessage> sub =
mDum.makeSubscription(mLastRequest->header(h_To), getEventType()); -
mDum.send(sub); - } - else - { -
SharedPtr<SipMessage> sub = mDum.makeSubscription(mDialog.mRemoteTarget,
getEventType(), getAppDialogSet()->reuse()); -
mDum.send(sub); - } - //!dcm! -- new
sub created above, when does this usage get destroyed? +
SharedPtr<SipMessage> sub =
mDum.makeSubscription(mLastRequest->header(h_To), getEventType()); +
mDum.send(sub); + +
//!dcm! -- new sub created above, when does this usage get destroyed? //return; } else @@ -386,17 +379,8 @@ else {
InfoLog(<< "ClientSubscription: application retry new
request"); - -
if (mDialog.mRemoteTarget.uri().host().empty()) -
{ -
SharedPtr<SipMessage> sub =
mDum.makeSubscription(mLastRequest->header(h_To), getEventType()); -
mDum.send(sub); -
} -
else -
{ -
SharedPtr<SipMessage> sub = mDum.makeSubscription(mDialog.mRemoteTarget,
getEventType(), getAppDialogSet()->reuse()); -
mDum.send(sub); -
} +
SharedPtr<SipMessage> sub =
mDum.makeSubscription(mLastRequest->header(h_To), getEventType()); +
mDum.send(sub);
delete this; } From: Byron Campen
[mailto:bcampen@xxxxxxxxxxxx]
That is _completely_ broken. It's not even close to being correct. This
needs to be fixed. Best regards, Byron Campen In ClientSubscription.cxx mRemoteTarget is used as the first parameter
to makeSubscription, which is populated from the Contact header whenever
onRequestRetry returns 0. -Aron From: Byron
Campen [mailto:bcampen@xxxxxxxxxxxx] DUM is using the value of the Contact header in the To
header? What? Best regards, Byron Campen
I get an empty To/From since the presence server (OpenSER) puts only the ip:port or domain name as the
Contact header in the dialog. Contact: <sip:sip.sightspeed.com> or
<sip:123.123.123.123:5060> If the dialog/transaction then times-out (408) the onRequestRetry
callback is called. Our code returns 0 (please retry transaction). In this case
the mRemoteTarget contained the original contact header (just domain name or
ip:port) and this value is then used as the new “To” header when the
ClientSubscription.cxx code calls makeSubscription for us. We then have a
To/From header which is missing the “user” field of the original request. As for the “end”. We call end off the subscription handle at
some point in the future (i.e. when the user is logging out) -Aron From: Scott
Godin [mailto:slgodin@xxxxxxxxxxxx] The NULL mAppDialogSet pointer issues aside – I’m a little
confused how you are getting the empty From/To header. From the code it
seems that if mRemoteTarget is set (ie. Host is non-empty) then it will use
that, otherwise it will use mLastRequest->header(h_To). How does it
end up being empty? Also – from your step 2 below, you say: Ø Client
ends the subscription by invoking end() on the handle What handle are you referring to? I don’t think you should
even have a client subscription handle until the first notify arrives and you
get your first callback (onNewSubscription). Scott From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of Aron Rosenberg Here is a simple patch which I believe addresses part of the
issue we are seeing. Index: resip/dum/ClientSubscription.cxx =================================================================== --- resip/dum/ClientSubscription.cxx
(revision 8133) +++ resip/dum/ClientSubscription.cxx (working copy) @@ -78,10 +78,7 @@ if
(!mOnNewSubscriptionCalled && !getAppDialogSet()->isReUsed()) { InfoLog
(<< "[ClientSubscription] " <<
mLastRequest->header(h_To)); - if
(msg.exists(h_Contacts)) - { -
mDialog.mRemoteTarget = msg.header(h_Contacts).front(); - } +
mDialog.mRemoteTarget = msg.header(h_To);
handler->onNewSubscription(getHandle(), msg);
mOnNewSubscriptionCalled = true; -- The original symptom of an empty From/To header was caused by
the mRemoteTarget being set with the contact address which is almost always (IP:Port)
or just (DNS name:port). The mRemoteTarget was then used to build the
resubscribe if you requested it which resulted in the empty to/from username. I believe that all the if…else cases which tested mRemoteTarget
for Uri / Host values aren’t needed if the above is fixed properly. There is still the issue that the getAppDialogSet() crashes
since the pointer is NULL -Aron From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of Aron Rosenberg I was finally able to get a working pcap, resip log and
debug crash at the same time. Here is what is going on 1. Client makes subscription 2. Client ends the subscription
by invoking end() on the handle 3. This end results in a local
408 error, which calls onRequestRetry 4. Our code returns 0 to
onRequestRetry(ClientSubscriptionHandle) to retry the request since we want the
server to know we ended the sub 5. "Application requested immediate
retry on Retry-After" is printed to
log 6. Crash happens in the else
statement in ClientSubscription.cxx:198 when trying to call getAppDialogSet()->reuse(). I have a full log (over 100MB of resip data which I can send to
a developer who wants to look at it along with the matching pcap error file -Aron From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of Aron Rosenberg Here is a little bit more information gleaned from a pcap trace. The stack seems to be crashing when dealing with a 400 error
where the “From:” header looks like this “From: <sip:>;tag=5b461e50” I was able to find the outbound SUBSCRIBE request and it also
has an empty From address so something strange is going on in the stack. Still
working on getting the resip logs. -Aron From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of Aron Rosenberg Resip ver: SVN rev 8128 on 1.3 branch Call Stack: resip::AppDialogSet::getHandle() Line 22 + 0x3 bytes C++ The crash is because the appDialogSet returned in
DialogUsage::getAppDialogSet() is NULL. It came from our production client and is reasonable repeatable,
so I am working on getting the resip logs that would go with it. -Aron --------------------------------------------- Aron
Rosenberg Founder
and CTO SightSpeed
- http://www.sightspeed.com/ 918
Parker St, Suite A14 Berkeley,
CA 94710 Email: arosenberg@xxxxxxxxxxxxxx Phone:
510-665-2920 Cell:
510-847-7389 Fax:
510-649-9569 SightSpeed
Video Link: http://aron.sightspeed.com _______________________________________________ |