[OpenSIPS-Devel] presentity expiration and etag handling

2010-09-02 Thread Kennard_White


Hi,

While looking at another feature (see patch upload 3058434) I came across a
strange behavior and traced it down to a section of  presence code.

Given an subscription (via SUBSCRIBE) and an existing presentity (via
PUBLISH) for event "presence", opensips send different NOTIFY PIDFs
documents depending upon if the presentity is expired via timeout vs an
explicit PUBLISH with Expires=0 header. In the first case (timeout), the
NOTIFY PIDF is completely missing the original presentity (and the body may
be empty if no other tuples). In the second case (unpublish), the NOTIFY
PIDF has an edited copy of the original presentity (with basic status set
closed). This occurs when the presentity hash tables are enabled and
fallback2db is disabled.

I doubt this is a really a bug (the behavior when presentities go away is
not defined by the RFCs as far as I can tell). But I suspect this is a
change in behavior introduced by the hash table code. Since some clients
react very differently to no-body NOTIFY messages compared to ones
containing 'closed' status, this can (and did) lead to strange system
behavior.

The root cause of the different behavior is that during message cleaning,
the presentity is deleted from the hash tables before the notification is
generated. In more detail, the call sequence is:

publish.c msg_presentity_clean() calls
notify.c: publ_notify() calls
  notify.c: get_p_notify_body() calls
notify_body.c: pres_agg_nbody()

The etag of the expiring presentity (or its body index) is passed down the
above call chain. The last function attempts to "edit" the expiring
presentity, setting the basic status to 'closed' prior to sending out the
NOTIFY.

But the stale presentity is deleted from the hash table before the above
call sequence, thus it never shows up in the list of presentities queried
by get_p_notify_body() and is not edited to indicate closed.

Probably msg_presentity_clean() calling publ_notify() with the expiring
etag doesn't serve any purpose, at least with fallback2db disabled, and is
potentially misleading since it doesn't do what one would expect.

Regards,
Kennard___
Devel mailing list
Devel@lists.opensips.org
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel


Re: [OpenSIPS-Devel] presentity expiration and etag handling

2010-09-03 Thread Anca Vamanu

Hi Kennard,

Yes, you are right, I should not delete the htable entry before calling 
the publ_notify function. I will fix this. Thanks for pointing out.


Regards,

--
Anca Vamanu
www.voice-system.ro



On 09/03/2010 02:37 AM, kennard_wh...@logitech.com wrote:


Hi,

While looking at another feature (see patch upload 3058434) I came 
across a strange behavior and traced it down to a section of presence 
code.


Given an subscription (via SUBSCRIBE) and an existing presentity (via 
PUBLISH) for event "presence", opensips send different NOTIFY PIDFs 
documents depending upon if the presentity is expired via timeout vs 
an explicit PUBLISH with Expires=0 header. In the first case 
(timeout), the NOTIFY PIDF is completely missing the original 
presentity (and the body may be empty if no other tuples). In the 
second case (unpublish), the NOTIFY PIDF has an edited copy of the 
original presentity (with basic status set closed). This occurs when 
the presentity hash tables are enabled and fallback2db is disabled.


I doubt this is a really a bug (the behavior when presentities go away 
is not defined by the RFCs as far as I can tell). But I suspect this 
is a change in behavior introduced by the hash table code. Since some 
clients react very differently to no-body NOTIFY messages compared to 
ones containing 'closed' status, this can (and did) lead to strange 
system behavior.


The root cause of the different behavior is that during message 
cleaning, the presentity is deleted from the hash tables before the 
notification is generated. In more detail, the call sequence is:


publish.c msg_presentity_clean() calls
notify.c: publ_notify() calls
notify.c: get_p_notify_body() calls
notify_body.c: pres_agg_nbody()

The etag of the expiring presentity (or its body index) is passed down 
the above call chain. The last function attempts to "edit" the 
expiring presentity, setting the basic status to 'closed' prior to 
sending out the NOTIFY.


But the stale presentity is deleted from the hash table before the 
above call sequence, thus it never shows up in the list of 
presentities queried by get_p_notify_body() and is not edited to 
indicate closed.


Probably msg_presentity_clean() calling publ_notify() with the 
expiring etag doesn't serve any purpose, at least with fallback2db 
disabled, and is potentially misleading since it doesn't do what one 
would expect.


Regards,
Kennard

___
Devel mailing list
Devel@lists.opensips.org
http://lists.opensips.org/cgi-bin/mailman/listinfo/devel