possible memory leaks
Moderators: gto, cipto_kh, EgonHugeist
possible memory leaks
Currently i am using zeoslib with an sqlite database and instant objects. When searching for memoryleaks in my application using fastmm i also saw some memory leaks comming from zeoslib dealing with the sqlite database connections.
Is this known already, or else where should i repport this and how?
Is this known already, or else where should i repport this and how?
- mdaems
- Zeos Project Manager
- Posts: 2766
- Joined: 20.09.2005, 15:28
- Location: Brussels, Belgium
- Contact:
It is 'known', but the solutions and reasons are not. So, unless you can identify the reason of the leaks you don't have to report.
If you can find exactly what memory is leaking away and eventually how to resolve this, please track a bug report on http://zeosbugs.firmos.at/
Groetjes,
Mark
If you can find exactly what memory is leaking away and eventually how to resolve this, please track a bug report on http://zeosbugs.firmos.at/
Groetjes,
Mark
After reading http://zeos.firmos.at/viewtopic.php?t=1339 I can hardly believe nobody found this somewhat obvious leak : function TZSQLite3PlainDriver.Step allocates memory which is never freed.
Here is my (quick) fix - maybe someone with a killer application can test it (how about Orbmu2k testing it with his superb sqlite administrator ?)
and
I hope someone can check this into SVN (I tried, but don't have user/pw).
Here is my (quick) fix - maybe someone with a killer application can test it (how about Orbmu2k testing it with his superb sqlite administrator ?)
Code: Select all
Unit ZDbcSqLiteResultSet;
...
function TZSQLiteResultSet.Next: Boolean;
var
ErrorCode: Integer;
begin
{ Checks for maximum row. }
Result := False;
if (MaxRows > 0) and (RowNo >= MaxRows) then
Exit;
if LastRowNo = 0 then
begin
Result := FColumnValues <> nil;
if Result then
begin
LastRowNo := LastRowNo + 1;
RowNo := RowNo + 1;
end
else
begin
if RowNo <= LastRowNo then
RowNo := LastRowNo + 1;
end;
end
else
begin
//ZPlainSQLLiteDriver.Step : AllocMem(SizeOf(PPChar)*pN+1); // Leak, if not freed ! [HD, 05.10.2007]
if FColumnValues <> nil then
FreeMem(FColumnValues,Sizeof(PPChar)*fColumnCount+1);
FColumnValues := nil;
//ZPlainSQLLiteDriver.Step : AllocMem(SizeOf(PPChar)*pN*2+1); // Leak, if not freed [HD, 05.10.2007]
if FColumnNames <> nil then
FreeMem(FColumnNames,Sizeof(PPChar)*fColumnCount*2+1);
FColumnNames := nil;
if Assigned(FStmtHandle) then
begin
ErrorCode := FPlainDriver.Step(FStmtHandle, FColumnCount,
FColumnValues, FColumnNames);
CheckSQLiteError(FPlainDriver, ErrorCode, nil, lcOther, 'FETCH');
end;
if FColumnValues <> nil then
begin
RowNo := RowNo + 1;
if LastRowNo < RowNo then
LastRowNo := RowNo;
Result := True;
end
else
begin
if RowNo <= LastRowNo then
RowNo := LastRowNo + 1;
Result := False;
end;
end;
{ Frees handle when reads to the end. }
if not Result and Assigned(FStmtHandle) then
FreeHandle;
end;
Code: Select all
destructor TZSQLiteResultSet.Destroy;
begin
//ZPlainSQLLiteDriver.Step : AllocMem(SizeOf(PPChar)*pN+1); // Leak, if not freed ! [HD, 05.10.2007]
if FColumnValues <> nil then
FreeMem(FColumnValues,Sizeof(PPChar)*fColumnCount+1);
FColumnValues := nil;
//ZPlainSQLLiteDriver.Step : AllocMem(SizeOf(PPChar)*pN*2+1); // Leak, if not freed ! [HD, 05.10.2007]
if FColumnNames <> nil then
FreeMem(FColumnNames,Sizeof(PPChar)*fColumnCount*2+1);
FColumnNames := nil;
inherited Destroy;
end;
- mdaems
- Zeos Project Manager
- Posts: 2766
- Joined: 20.09.2005, 15:28
- Location: Brussels, Belgium
- Contact:
Thanks Spongebob.
I added this topic to my personal (=not official) 'todo' list. I hope somebody can test it and replies. (aducom/orbmu2k??)
As soon as I can get a serious confirmation it will be added immediately to SVN. Otherwise we'll have to wait until I can afford to spend some time with SQLite.
Mark
I added this topic to my personal (=not official) 'todo' list. I hope somebody can test it and replies. (aducom/orbmu2k??)
As soon as I can get a serious confirmation it will be added immediately to SVN. Otherwise we'll have to wait until I can afford to spend some time with SQLite.
Mark
- mdaems
- Zeos Project Manager
- Posts: 2766
- Joined: 20.09.2005, 15:28
- Location: Brussels, Belgium
- Contact:
spongebob,
I added sqlite to my test suite runs and ran the suite before and after your changes. I had to adapt your code a little (I had loads of pointer exceptions because in the 'Next' procedure these variables are cleared, even when Step isn't executed.) After the changes these exceptions where solved.
See SVN commit 298
Mark
I added sqlite to my test suite runs and ran the suite before and after your changes. I had to adapt your code a little (I had loads of pointer exceptions because in the 'Next' procedure these variables are cleared, even when Step isn't executed.) After the changes these exceptions where solved.
See SVN commit 298
Mark
You are right. I did not run into any situation where FStmtHandle was nil, so I did not experience the exceptions.mdaems wrote:spongebob,
I added sqlite to my test suite runs and ran the suite before and after your changes. I had to adapt your code a little (I had loads of pointer exceptions because in the 'Next' procedure these variables are cleared, even when Step isn't executed.) After the changes these exceptions where solved.
See SVN commit 298
Mark
Have a nice day !
Rev 298 still contains a leak :
in TZSQLiteResultSet.Next :
In this code, the memory referenced by FColumnValues will never get freed because it is set to nil before checking the FStmtHandle.
I'm still not really sure, in what situation FStmtHandle would be nil. (Can you point me to some code ?)
I need to investigate this a little further - I'll be back.
in TZSQLiteResultSet.Next :
Code: Select all
FColumnValues := nil;
if Assigned(FStmtHandle) then
begin
//ZPlainSQLLiteDriver.Step : AllocMem(SizeOf(PPChar)*pN+1); // Leak, if ...not freed ! [HD, 05.10.2007]
if FColumnValues <> nil then
FreeMem(FColumnValues,Sizeof(PPChar)*fColumnCount+1);
FColumnValues := nil;
//ZPlainSQLLiteDriver.Step : AllocMem(SizeOf(PPChar)*pN*2+1); // Leak, if not freed [HD, 05.10.2007]
if FColumnNames <> nil then
FreeMem(FColumnNames,Sizeof(PPChar)*fColumnCount*2+1);
FColumnNames := nil;
ErrorCode := FPlainDriver.Step(FStmtHandle, FColumnCount,
FColumnValues, FColumnNames);
CheckSQLiteError(FPlainDriver, ErrorCode, nil, lcOther, 'FETCH');
end;
I'm still not really sure, in what situation FStmtHandle would be nil. (Can you point me to some code ?)
I need to investigate this a little further - I'll be back.
- mdaems
- Zeos Project Manager
- Posts: 2766
- Joined: 20.09.2005, 15:28
- Location: Brussels, Belgium
- Contact:
Have you ever used the test suite? It's not that difficult to compile and run. Once you get it running wit current version and you change back to your original bugfix, you'll see the exceptions pop up.
Should I try moving the FColumnValues Freemem before the procedure above? I suppose : when you can nil it, you can freemem it?
Mark
Should I try moving the FColumnValues Freemem before the procedure above? I suppose : when you can nil it, you can freemem it?
Mark
I just checked out Rev 303 and the leaks are gone - so if it the current Revision passes your test suite everything should be ok.mdaems wrote:What about Revision 301??
(Sorry, but installing cygwin/ant/ .... was beyond my scope. While I realize you have to use a framework like this (platform portability) I'd prefer DUnit which is part of your files but is not used (??) ).
BTW : I tried to fix the leak in function TZSQLite3PlainDriver.Step by freeing var pazValue, pazColName: PPChar but for some Reason freemem() did not work as expected - so TZSQLiteResultSet.Next seems to be the right place to fix it.
Thanks for looking into this !
- mdaems
- Zeos Project Manager
- Posts: 2766
- Joined: 20.09.2005, 15:28
- Location: Brussels, Belgium
- Contact:
cygwin? I just installed ant on my windows box, but that's just necessary for doing the clean/build/build tests/run test things as one procedure. (scripts in build directory)
You should use the ZeosDboDevel.bpg and just
- Build and install the components
- Build and install the framework project
- Build the test executables
- Run the test executables from the packages\delphi7\build directory after setting database\test.properties to your database settings (also make sure the sqlite dll is available)
The only tricky part is making sure the right versions of components and testframework are used by the testprograms. I found that it's necessary to build+install components, then build/install framework and then build all test applications.
When you use the ant build script that's done automatically as it cleans everything before building. But it's not necessary.
If you find some time to try it, it would be nice if you had a look at the sqlite specific errors the test suite shows. I have no experience with sqlite, so it would take me a lot of time to fix those myself.
And finally : the testframework is just some advanced wrapper around DUnit.
You should use the ZeosDboDevel.bpg and just
- Build and install the components
- Build and install the framework project
- Build the test executables
- Run the test executables from the packages\delphi7\build directory after setting database\test.properties to your database settings (also make sure the sqlite dll is available)
The only tricky part is making sure the right versions of components and testframework are used by the testprograms. I found that it's necessary to build+install components, then build/install framework and then build all test applications.
When you use the ant build script that's done automatically as it cleans everything before building. But it's not necessary.
If you find some time to try it, it would be nice if you had a look at the sqlite specific errors the test suite shows. I have no experience with sqlite, so it would take me a lot of time to fix those myself.
And finally : the testframework is just some advanced wrapper around DUnit.