[sqlite] Two-line patch to fix memory leak

2012-05-11 Thread Eric Sink


In sqlite3Fts3Matchinfo():

Near the end of the function, I added two lines:

  if( rc!=SQLITE_OK ){
sqlite3_result_error_code(pContext, rc);
  }else{
int n = pCsr->nMatchinfo * sizeof(u32);
sqlite3_result_blob(pContext, pCsr->aMatchinfo, n, SQLITE_TRANSIENT);
ADDED:sqlite3_free(pCsr->aMatchinfo);
ADDED:   pCsr->aMatchinfo = 0;
  }

Notes:

1.  I added these two lines while obtaining zero actual understanding of 
the internals of sqlite.  Like a monkey typing random characters, I 
opened vi and hit keys until the leak went away.  I have no reason to 
believe these lines are correct.  All I know is that our test suite 
shows some memory leaks unless these two lines are present.


2.  I first did this fix back when 3.7.5 was released.  And then I 
procrastinated about submitting the patch to the mailing list, because I 
wanted to wait until I could say something insightful about sqlite 
internals, to make me look smarter than the aforementioned monkey.  
Anyway, we've got over a year of experience running with "sqlite 3.7.5 
plus this patch", and no problems.


3.  This week we upgraded to 3.7.11 and the leaks showed up again.  So I 
reapplied this little patch, and the leaks went away again.  And this 
time, I have decided not to let my cluelessness prevent me from letting 
you folks know about it.


So, FWIW, there it is.  :-)

--
E

___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users


Re: [sqlite] Two-line patch to fix memory leak

2012-05-11 Thread Dan Kennedy

On 05/11/2012 11:28 PM, Eric Sink wrote:


In sqlite3Fts3Matchinfo():

Near the end of the function, I added two lines:

if( rc!=SQLITE_OK ){
sqlite3_result_error_code(pContext, rc);
}else{
int n = pCsr->nMatchinfo * sizeof(u32);
sqlite3_result_blob(pContext, pCsr->aMatchinfo, n, SQLITE_TRANSIENT);
ADDED: sqlite3_free(pCsr->aMatchinfo);
ADDED: pCsr->aMatchinfo = 0;
}


Thanks for posting this.

Looking at the code, I can't see how the memory leak occurs. Do
you have any idea how to trigger it? Other than this patch, are
you using unmodified 3.7.11 FTS code?

We can't apply the patch as is, even though it is safe (does not
introduce any bugs), because it makes queries that use the 'x'
format specifier with matchinfo() much less efficient.

Dan.
___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users


Re: [sqlite] Two-line patch to fix memory leak

2012-05-11 Thread Eric Sink

Oh darn.

If the leak is not obvious to you, and if my patch would cause an undesirable 
side effect, then the leak may somehow be the fault of our code.

I'll have to dig into this a bit further.  Sending you our whole tree would be 
somewhat less than constructive.  Maybe I can narrow down to a small test case.

For now, I can confirm that we have made no other changes to our copy of 3.7.11.

Thanks,

--
E


On May 11, 2012, at 12:50 PM, Dan Kennedy  wrote:

> On 05/11/2012 11:28 PM, Eric Sink wrote:
>> 
>> In sqlite3Fts3Matchinfo():
>> 
>> Near the end of the function, I added two lines:
>> 
>> if( rc!=SQLITE_OK ){
>> sqlite3_result_error_code(pContext, rc);
>> }else{
>> int n = pCsr->nMatchinfo * sizeof(u32);
>> sqlite3_result_blob(pContext, pCsr->aMatchinfo, n, SQLITE_TRANSIENT);
>> ADDED: sqlite3_free(pCsr->aMatchinfo);
>> ADDED: pCsr->aMatchinfo = 0;
>> }
> 
> Thanks for posting this.
> 
> Looking at the code, I can't see how the memory leak occurs. Do
> you have any idea how to trigger it? Other than this patch, are
> you using unmodified 3.7.11 FTS code?
> 
> We can't apply the patch as is, even though it is safe (does not
> introduce any bugs), because it makes queries that use the 'x'
> format specifier with matchinfo() much less efficient.
> 
> Dan.
> ___
> sqlite-users mailing list
> sqlite-users@sqlite.org
> http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
___
sqlite-users mailing list
sqlite-users@sqlite.org
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users