You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directory.apache.org by Alex Karasulu <ak...@apache.org> on 2008/10/14 01:17:11 UTC
Re: Problem in SearchHandler in AD 1.5.4 ?
On Wed, Sep 17, 2008 at 3:01 AM, Norval Hope <nr...@gmail.com> wrote:
> Hi All,
>
> Just had a question about SearchHandler as I'm reviewing to see if
> some memory leaks I experienced in the old <1.5.0 release code base
> have been resolved. As best I can tell (apologies if I'm missing
> something) there seem to be some different probs in the new
> implementation regarding calling of
> session.unregisterOutstandingRequest( req ) (the old impl had some
> probs in the similar but now defunct use of SessionRegistry):
> 1. Note that SearchHandler.handleIgnoringReferrals() always calls
> session.registerOutstandingRequest( req ) immediately on entry
Yes this should happen.
>
> 2. When it delegates to handleRootDseSearch() i don't see a
> corresponding session.unregisterOutstandingRequest( req ) call, isn't
> one required?
Yes you are right it should be required. This is a bug which will cause a
leak.
>
> 3. I'm not sure what the right behaviour is for
> handlePersistentSearch(), but I'd expect that logically
> session.unregisterOutstandingRequest( req ) would need to be called
> too (or in this case is the req considered to be outstanding
> indefinitely?)
Right it need not be called in the case of psearch. Here the only thing
that will stop it is an abandon or the destruction of the
connection/session.
>
> 4. In the other normal search cases, shouldn't
> session.unregisterOutstandingRequest( req ) be in a finally block so
> it is also called when exceptions are encountered?
Yes indeed you are right again. Another lousy leak.
>
> 5. The old impl had a memory leak for searches when no results were
> returned, as best I can tell the new cursor stuff doesn't suffer the
> same problem but just wanted to throw this boundary case out there for
> special consideration.
>
Best thing to do is write some test cases and see if we can produce a leak
for this boundary case. Should be pretty easily done.
FYI once we move to MINA 2.0 we can start looking at the slow client problem
as well as hunting down more leaks in preparation for ADS 2.0.
Thanks,
Alex
Re: Problem in SearchHandler in AD 1.5.4 ?
Posted by Alex Karasulu <ak...@apache.org>.
Sorry about that - been out of the loop these days.
Regards,
Alex
On Tue, Oct 14, 2008 at 10:40 PM, Norval Hope <nr...@gmail.com> wrote:
> Thanks for the confirmation Alex. FYI Emmanuel had already worked with
> me to fix the issue a while ago.
>
> On Tue, Oct 14, 2008 at 10:17 AM, Alex Karasulu <ak...@apache.org>
> wrote:
> >
> >
> > On Wed, Sep 17, 2008 at 3:01 AM, Norval Hope <nr...@gmail.com> wrote:
> >>
> >> Hi All,
> >>
> >> Just had a question about SearchHandler as I'm reviewing to see if
> >> some memory leaks I experienced in the old <1.5.0 release code base
> >> have been resolved. As best I can tell (apologies if I'm missing
> >> something) there seem to be some different probs in the new
> >> implementation regarding calling of
> >> session.unregisterOutstandingRequest( req ) (the old impl had some
> >> probs in the similar but now defunct use of SessionRegistry):
> >> 1. Note that SearchHandler.handleIgnoringReferrals() always calls
> >> session.registerOutstandingRequest( req ) immediately on entry
> >
> > Yes this should happen.
> >
> >>
> >> 2. When it delegates to handleRootDseSearch() i don't see a
> >> corresponding session.unregisterOutstandingRequest( req ) call, isn't
> >> one required?
> >
> > Yes you are right it should be required. This is a bug which will cause
> a
> > leak.
> >
> >>
> >> 3. I'm not sure what the right behaviour is for
> >> handlePersistentSearch(), but I'd expect that logically
> >> session.unregisterOutstandingRequest( req ) would need to be called
> >> too (or in this case is the req considered to be outstanding
> >> indefinitely?)
> >
> > Right it need not be called in the case of psearch. Here the only thing
> > that will stop it is an abandon or the destruction of the
> > connection/session.
> >
> >>
> >> 4. In the other normal search cases, shouldn't
> >> session.unregisterOutstandingRequest( req ) be in a finally block so
> >> it is also called when exceptions are encountered?
> >
> > Yes indeed you are right again. Another lousy leak.
> >
> >>
> >> 5. The old impl had a memory leak for searches when no results were
> >> returned, as best I can tell the new cursor stuff doesn't suffer the
> >> same problem but just wanted to throw this boundary case out there for
> >> special consideration.
> >
> > Best thing to do is write some test cases and see if we can produce a
> leak
> > for this boundary case. Should be pretty easily done.
> >
> > FYI once we move to MINA 2.0 we can start looking at the slow client
> problem
> > as well as hunting down more leaks in preparation for ADS 2.0.
> >
> > Thanks,
> > Alex
> >
> >
>
Re: Problem in SearchHandler in AD 1.5.4 ?
Posted by Emmanuel Lecharny <el...@gmail.com>.
Norval Hope wrote:
> Thanks for the confirmation Alex. FYI Emmanuel had already worked with
> me to fix the issue a while ago.
>
Yes, thanks to Norval, who pointed out this very nasty bug, and proposed
some patches, it was fixed last month, I think.
--
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org
Re: Problem in SearchHandler in AD 1.5.4 ?
Posted by Norval Hope <nr...@gmail.com>.
Thanks for the confirmation Alex. FYI Emmanuel had already worked with
me to fix the issue a while ago.
On Tue, Oct 14, 2008 at 10:17 AM, Alex Karasulu <ak...@apache.org> wrote:
>
>
> On Wed, Sep 17, 2008 at 3:01 AM, Norval Hope <nr...@gmail.com> wrote:
>>
>> Hi All,
>>
>> Just had a question about SearchHandler as I'm reviewing to see if
>> some memory leaks I experienced in the old <1.5.0 release code base
>> have been resolved. As best I can tell (apologies if I'm missing
>> something) there seem to be some different probs in the new
>> implementation regarding calling of
>> session.unregisterOutstandingRequest( req ) (the old impl had some
>> probs in the similar but now defunct use of SessionRegistry):
>> 1. Note that SearchHandler.handleIgnoringReferrals() always calls
>> session.registerOutstandingRequest( req ) immediately on entry
>
> Yes this should happen.
>
>>
>> 2. When it delegates to handleRootDseSearch() i don't see a
>> corresponding session.unregisterOutstandingRequest( req ) call, isn't
>> one required?
>
> Yes you are right it should be required. This is a bug which will cause a
> leak.
>
>>
>> 3. I'm not sure what the right behaviour is for
>> handlePersistentSearch(), but I'd expect that logically
>> session.unregisterOutstandingRequest( req ) would need to be called
>> too (or in this case is the req considered to be outstanding
>> indefinitely?)
>
> Right it need not be called in the case of psearch. Here the only thing
> that will stop it is an abandon or the destruction of the
> connection/session.
>
>>
>> 4. In the other normal search cases, shouldn't
>> session.unregisterOutstandingRequest( req ) be in a finally block so
>> it is also called when exceptions are encountered?
>
> Yes indeed you are right again. Another lousy leak.
>
>>
>> 5. The old impl had a memory leak for searches when no results were
>> returned, as best I can tell the new cursor stuff doesn't suffer the
>> same problem but just wanted to throw this boundary case out there for
>> special consideration.
>
> Best thing to do is write some test cases and see if we can produce a leak
> for this boundary case. Should be pretty easily done.
>
> FYI once we move to MINA 2.0 we can start looking at the slow client problem
> as well as hunting down more leaks in preparation for ADS 2.0.
>
> Thanks,
> Alex
>
>