[JDEV] Leak, bug -- fixed -- maybe

[Glenn MacGregor]

----- Original Message -----
From: "Jeremie" <jeremie at jabber.org>
To: "Glenn MacGregor" <gtm at oracom.com>
Cc: <jdev at jabber.org>; <jabberd at jabberstudio.org>
Sent: Tuesday, April 02, 2002 1:44 AM
Subject: Re: [JDEV] Leak, bug -- fixed -- maybe


> Hey Glenn, few questions to clarify here, and following up to the
> jabberd at jabberstudio.org list as well.
>
> > jsm/session.c at _js_session_from you will see at line 252 or so that we
> > create a new jid struct using jid_full with the session->id meaning that
the
> > new jid is allocated from the session pool, not good.
>
> Are you talking about this code? (and the line marked with ->)
>
>     /* if you use to="yourself at yourhost" it's the same as not having a ...
*/
> ->  uid = jid_user(s->id);
>     if(jid_cmp(p->to,uid) == 0)
>     {
>         /* xmlnode_hide_attrib(p->x,"to"); */
>         p->to = NULL;
>     }
>
> Just curious, because yes, that is a leak with using the jid_user()
> function on the session id... but from your wording:
>
> > So we have created a new jid_full function, jid_full_from_pool(pool,
jid).
>
> ... it sounds like you are saying a call to jid_full(s->id) would leak,
> and it doesn't because the implementation in jid.c 'caches' the results:
>
>     /* use cached copy */
>     if(id->full != NULL)
>         return id->full;
>
> So it is safe to use jid_full() many numbers of times on a "static" jid
> like the one in the session.

Sorry for that wording, but the main leak in the jid_user function using the
session pool.  So I noticed in 1.4.2 you added the jid_cmpx function, I just
replaced:
    uid = jid_user(s->id);
    if(jid_cmp(p->to,uid) == 0)

with:
    if (jid_cmpx(p->to, s->id,JID_USER|JID_SERVER)) {

I guess I am not quite sure of the call stack getting to _js_session_from, I
was thinking that it would be different for each sender.  Now I see that is
not the case.

>
> > changed much so if the plan was to reuse some of the jsm and jabberd
stuff
> > in the 1.5 it maybe good to get it in.
>
> Yes, much of the jsm code is going to be reused, at least initially to get
> 1.5 rolling. Since you pointed this out I looked at other suspect uses of
> jid_user, and there are numerous other problems in the modules (my bad)
> that I'll look at and make sure are fixed for 1.5.
>
> If you want to work up a patch without having to figure out where it's
> used incorrectly in jsm, the easiest fix for jid_user might be to make it
> work like jid_full, cache the returned user-only jid in the struct so that
> it is only ever generated once.  Just post it to the jdev and jabberd
> lists, if you plan on having any more patches then cvs access is available
> as well (just ask).
I don't think you want to malloc anything on the session pool, because that
pool may never be freed.  It is definatly a toss up, do you do the work
every time or do you cache result.  There is no easy answer for that one.
We use the jabber server to pass about 100 messages a second with 700-800
users logged in, it is a very regirious testing ground.

I will take out the jid_full_from_pool and see how that effects it and post
my current patch.  I will also remake jid_user and run the tests.

Like you will see in the patch, the way we got around the jid_user problem
is by removing it totally and using jid_cmpx instead of creating the jid and
using jid_cmp.
>
> Thanks for finding this, so far 1.5 is getting a pretty serious overhaul
> and it'll be easier for us all to track down and fix some of these
> problems :)
>
> Jer
>
>
>