possible memory leaks

[mvdhoning]

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?

[mdaems]

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

[aducom]

I've been looking at this but couldn't actually find something. I thought it had to do with the lazaras package?

[spongebob]

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 ?)

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;

and

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;

I hope someone can check this into SVN (I tried, but don't have user/pw).

[mdaems]

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

[mdaems]

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

[spongebob]

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

You are right. I did not run into any situation where FStmtHandle was nil, so I did not experience the exceptions.

Have a nice day !

[spongebob]

Rev 298 still contains a leak :

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;

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.

[mdaems]

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

[mdaems]

What about Revision 301??

[spongebob]

What about Revision 301??

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.

(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]

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.