Sorry about that - been out of the loop these days. Regards, Alex
On Tue, Oct 14, 2008 at 10:40 PM, Norval Hope <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> > wrote: > > > > > > On Wed, Sep 17, 2008 at 3:01 AM, Norval Hope <[EMAIL PROTECTED]> 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 > > > > >
