From: Kostka B. <ko...@to...> - 2010-06-29 07:21:44
|
This is the same solution (except you use much more efficient way) I proposed in one of my previous mails: > To test this, I simply added (yes it might be done more efficient way using memset) > > for ( size_t i = this->postingsFreeCountDW; i< this->postingsFreeListDW.length; i++ ){ > this->postingsFreeListDW.values[i] = NULL; } and you told me this will introduce memory leaks. And there are some in cl_demo. So you think this is OK, or some additional exploration should be done? Borek > -----Original Message----- > From: Itamar Syn-Hershko [mailto:it...@co...] > Sent: Tuesday, June 29, 2010 12:35 AM > To: clu...@li... > Subject: Re: [CLucene-dev] vector subscript outofrangeexceptionduringindexing > > Alrighty, seems like I have nailed it. See below + attached patch. > > On 29/6/2010 12:39 AM, Kostka Bořivoj wrote: > > I'm quite sure the problem is in postingsFreeListDW management: > > > > The postings after postingsFreeCountDW are used somewhere (but are > > still here in a list). If you remove block of free postings in > > balanceRAM, you are removing before postingsFreeCountDW so an "empty > > block" is created inside postingsFreeListDW (at the end of free > > postings but before all used postings). Then if you return all/some used postings > back into list, you return them to the beginning of this empty block. This can cause > duplicated pointers in the list. > > > I agree there is an empty block, I am just not sure how this could cause duplicated > pointers? > > Also, claiming the culprit is postingsFreeListDW's memory management is less likely > since it most likely would have been noticed by Java users / devs already. > > > It can also be a reason, why postingsFreeCountDW< postingsFreeListDW.length in > the destructor. > > > I am not at all sure this is an actual problem. postingsFreeListDW can have more > allocated memory than needed (hence < length). > > Borek > > Anyway, I put a breakpoint on Array.h ln 139 and noticed it errored on > array item 5888 (0-base). This was also the value of > postingsFreeCountDW. What this means is the "gap" you were referring to > is actually in the tails of the postings array. Some code moving > postings around didn't bother at NULLifying unused positions. > > Instead of looking up this erroneous code, I added a memset call to > NULLify the tail of that array at the end of the destructor. It also > makes more sense performance-wise, and as long as no memory leaks are > introduced, I think it is safe to keep it that way. > > So, what we need now is to make sure this doesn't introduce new leaks, > and have this properly tested before checking this change in (all > cl_tests pass atm). I know cl_demo leaks but haven't had the time to see > where from - this could be related. Can you handle that? > > Itamar. |