[CLucene-dev] [ clucene-Bugs-1047474 ] SegmentsReader memory leak

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 454-5900

Bugs item #1047474, was opened at 2004-10-14 20:15
Message generated for change (Comment added) made by woodsplitter
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=558446&aid=1047474&group_id=80013

Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Jacquie (jacquieh)
Assigned to: Nobody/Anonymous (nobody)
Summary: SegmentsReader memory leak

Initial Comment:
SegmentsReader was not deleting its NormsCache so I 
added the following :

void SegmentsReader::closeNorms() {
		LOCK_MUTEX(norms_mutex);
		map<const 
char_t*,l_byte_t*,lucene::util::charCompare>::iterator 
itr = normsCache.begin();
			while (itr != normsCache.end
()) {

				delete []itr-
>second;

				itr ++;
			}
		UNLOCK_MUTEX(norms_mutex);
	}

which I call in 
	void SegmentsReader::doClose() {
	    LOCK_MUTEX(doClose_LOCK);
		for (int_t i = 0; i < readersLength; 
i++){
			readers[i]->close();
		}
		closeNorms();
	    UNLOCK_MUTEX(doClose_LOCK);
	}

I also added 

		void closeNorms();

to segmentsReader.h as a private method.

It seems to work ok but I may not know entirely what I 
am doing

----------------------------------------------------------------------

Comment By: David Rushby (woodsplitter)
Date: 2004-11-04 03:25

Message:
Logged In: YES 
user_id=414645

The reason is that my brief window of opportunity to work on
CLucene closed.  I might be able to spend some time on it
this weekend, but I have both commercial clients and another
two open source projects making urgent demands on my time.

If you're sufficiently interested (the volume of your bug
reports suggests that you are), you might ask Ben for
developer rights to the CLucene SourceForge project so that
you can make CVS commits yourself.  (I apologize to Ben if
I'm overstepping my bounds in making this suggestion; I am,
after all, just a contributor to this project, not a founder
or an administrator.)

----------------------------------------------------------------------

Comment By: John Wheeler (j_wheeler)
Date: 2004-11-03 04:56

Message:
Logged In: YES 
user_id=1149323

Is there a reason this fix is not yet in the CVS repository?

----------------------------------------------------------------------

Comment By: Ken Ensdorf (kensdorf)
Date: 2004-11-01 09:20

Message:
Logged In: YES 
user_id=671771

John- 
I think you are right.  I added that line to the initializer list for 
SegmentsReader.  I guess you can get the same effect by 
calling setDoDelete(), so either is probably ok.

----------------------------------------------------------------------

Comment By: John Wheeler (j_wheeler)
Date: 2004-11-01 06:19

Message:
Logged In: YES 
user_id=1149323

Ken

You mean:

normsCache.setDoDelete(false, DELETE_TYPE_DELETE_ARRAY);

changing into:

normsCache.setDoDelete(true, DELETE_TYPE_DELETE_ARRAY);

right? Or are you talking about code that ain't in the
repository yet?

----------------------------------------------------------------------

Comment By: David Rushby (woodsplitter)
Date: 2004-10-31 22:15

Message:
Logged In: YES 
user_id=414645

Are you (Jacquie) using version 0.8.12?  On 2004.08.13
(after 0.8.12's release, I guess), I added the following
code to SegmentsReader's constructor:
-----------------------------------------------------------
// 2004.08.13: Fixed Erik-Kay-reported failure to free
normsCache's *values*.
    normsCache.setDoDelete(false, DELETE_TYPE_DELETE_ARRAY);
    // The memory underlying normsCache's *keys* is managed
by the Term class
    // (char_t* Term::field), so normsCache mustn't free its
keys.
-----------------------------------------------------------

The "Erik-Kay-report" of which I spoke is here:
http://sourceforge.net/mailarchive/forum.php?thread_id=5293141&forum_id=33782

If I recall correctly, I reviewed the CLucene-internal code
that touches the char_t* field memory in order to verify
that a scenario such as the one Ken describes does not arise
within the CLucene itself.  As long as the Term that manages
the char_t* field memory survives at least as long as the
SegmentsReader, corruption shouldn't occur.  Even if my
memory of that code review and the correctness of its
conclusions can be trusted, I agree that it would be all too
easy for a client programmer to provoke such a scenario, so
unless Ben stops me, I'll implement Ken's suggestion.

----------------------------------------------------------------------

Comment By: Ken Ensdorf (kensdorf)
Date: 2004-10-15 11:55

Message:
Logged In: YES 
user_id=671771

Actually the normsCache, which is an instance of VoidMap, 
takes care of the deletion of its data upon destruction of the 
object.  This is specified in the call to the contructor in the 
initializer list of SegmentsReader.  Unfortunately the data in 
the map can get corrupted, since it's being loaded with string 
keys that may be deleted while still in use by the map.  The 
following code in the getNorms() method:

normsCache.put(const_cast<char_t*>(field), bytes);

should be changed to:

char_t* key = stringDuplicate(field);
normsCache.put(key, bytes);

Since we are now allocatiing the storage for the keys, we 
need to tell the map to delete the keys when it is destroyed, 
so, in the constructor's initializer list, change:

normsCache(false, util::DELETE_TYPE_NONE, 
true, util::DELETE_TYPE_DELETE_ARRAY)

to:

normsCache(true, util::DELETE_TYPE_DELETE_ARRAY, 
true, util::DELETE_TYPE_DELETE_ARRAY)

That should fix the problem.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=558446&aid=1047474&group_id=80013