mORMot Open Source

migajek

Member

Registered: 2010-10-01

Posts: 89

Update - memory corruption problem?

Hi,
it seems that most recent leaf (downloaded from Fossil today) has some memory corruption problem while calling "update" method.

here's the most simple example of application (sources) which throws AV on application closing, if "update" was called.
http://migajek.us.to/tmp/synopsesqlite-update-bug.zip

in my more complex case I get following error report (FastMM):

FastMM has detected an error during a FreeMem operation. The block footer has been corrupted. 

The block size is: 84

This block was allocated by thread 0xFCC, and the stack trace (return addresses) at the time was:
402A03 [System][@GetMem]
404959 [System][@NewAnsiString]
404F1D [System][@LStrSetLength]
4AB2EE [SQLite3Commons.pas][SQLite3Commons][Return][6387]
4AB5AD [SQLite3Commons.pas][SQLite3Commons][GetJSONObjectAsSQL][6462]
4AB6BD [SQLite3Commons.pas][SQLite3Commons][GetJSONObjectAsSQL][6480]
5611A5 [SQLite3.pas][SQLite3][TSQLRestServerDB.EngineUpdate][3634]
4B1BF8 [SQLite3Commons.pas][SQLite3Commons][TSQLRestServer.URI][11031]
5618A7 [SQLite3.pas][SQLite3][TSQLRestClientDB.URI][3870]
4B0221 [SQLite3Commons.pas][SQLite3Commons][TSQLRestClientURI.Update][10161]
5B2CEC [frmRecordListing.pas][frmRecordListing][TFrame2.EditRecord][92]

The block is currently used for an object of class: Unknown

The allocation number is: 4261

The current thread ID is 0xFCC, and the stack trace (return addresses) leading to this error is:
402A23 [System][@FreeMem]
4048CE [System][@LStrArrayClr]
5611F7 [SQLite3.pas][SQLite3][TSQLRestServerDB.EngineUpdate][3637]
4B1BF8 [SQLite3Commons.pas][SQLite3Commons][TSQLRestServer.URI][11031]
5618A7 [SQLite3.pas][SQLite3][TSQLRestClientDB.URI][3870]
4B0221 [SQLite3Commons.pas][SQLite3Commons][TSQLRestClientURI.Update][10161]
5B2CEC [frmRecordListing.pas][frmRecordListing][TFrame2.EditRecord][92]
5B2E4B [frmRecordListing.pas][frmRecordListing][TFrame2.btnEditClick][160]
4630DE [Controls][TControl.Click]
45A76D [StdCtrls][TButton.Click]
45A861 [StdCtrls][TButton.CNCommand]

followed by 'invalid pointer operation'.
Finally, the debugger shows following line

            OK := EngineUpdate(Table,ID,SentData);

SQLite3Commons.pas:11031

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,655

Website

Re: Update - memory corruption problem?

First thing:
In your code, you're declaring the published field as string.
Which is not allowed in the framework (at least before Delphi 2009).
You'll have to declare it as RawUTF8.

In the Return sub function, there is this check:

  Assert(P-pointer(result)=length(result));

So the error should not be in the allocation, but in the consumption...

I don't have the TMS components installed here, so I can't test it now.
I'll try to reproduce it later, on another computer.

migajek

Member

Registered: 2010-10-01

Posts: 89

Re: Update - memory corruption problem?

Ok, I have changed the declaration to RawUTF8 on all the fields.
I'm using Delphi 7. And the problem still occurs...

actually the problem is reproducible on my sqlite-framework-demo which as you know doesn't require TMS.

Last edited by migajek (2011-02-18 13:13:45)

migajek

Member

Registered: 2010-10-01

Posts: 89

Re: Update - memory corruption problem?

investigating the problem, I found out that it is something about "GetJSONObjectAsSQL", part called "if Update" in "Return" subroutine.

Changing that part to much slower but definitely "safe" code (see below) I've elminated the error described above ... but now there's a problem with freeing TSQLRestClientURI object. FastMM detects operation on freed object on shutdown.
See the stack trace to the error...

BTW commenting out the FreeAndNil(dbConnection) part results in memory leak but no errors ... seems like there's some "after-update" operation pending, finally called on finalization...

  if Update then begin
    // returns 'COL1="VAL1", COL2=VAL2' (UPDATE SET format)
    result:= '';
    for F := 1 to FieldsCount do begin
      result:= result + Fields^;
      if InlinedParams then begin
        result:= result + '=:(';
      end else begin
        result:= result + '=';
      end;
        result:= result + Values^;
      if InlinedParams then begin
        result:= result + '):,';
      end else begin
        result:= result + ',';
      end;
      inc(Fields);
      inc(Values);
    end;
    result[length(result)]:= ' ';

well that's all I found, I hope it helps you somehow

//////
further investigating
I found out that each edited record doesn't get unlocked right after edition. It's ID stays on the Model's list of Locked IDs. That is why the "calling operation on freed object" error occurs.
But also, because of this bug, I'm not able to edit same record twice during the application runtime.

Last edited by migajek (2011-02-18 21:11:28)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,655

Website

Re: Update - memory corruption problem?

What if you code this:

      if InlinedParams then begin
        PWord(P)^ := Ord(')')+Ord(':')shl 8;
        P[2] := ',';
        // PInteger(P)^ := Ord(')')+Ord(':')shl 8+Ord(',')shl 16; may corrupt mem
        inc(P,3);
      end else begin

The PInteger() was faster but was perhaps storing one byte AFTER the "normal" end of string.

I'll take a look at the UNLOCK leak.

Thanks for the feedback and great proposals with screenshots!

migajek

Member

Registered: 2010-10-01

Posts: 89

Re: Update - memory corruption problem?

it seems that the new code fixed that bug Thank you !!
Now the only thing left here is the record-not-being-unlocked bug

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,655

Website

Re: Update - memory corruption problem?

The locking problem is not a bug, this is by design.

You have to manually UNLOCK the record you've locked.
Because there is no way the framework knows when the locking is finished.

It was documented as such:

    {{ this constructor initializes the object as above, and fill its content
      from a client or server connection
     - if ForUpdate is true, the REST method is LOCK and not GET: it tries to lock
      the corresponding record, then retrieve its content; caller has to call
      UnLock() method after Value usage, to release the record }
    constructor Create(aClient: TSQLRest; aID: integer;
      ForUpdate: boolean=false); overload;

You use ForUpdate=true so you must add an unlock() call.

Here is the correct code (note the try...finally block which is necessary to force unlocking):

procedure TForm1.Button2Click(Sender: TObject);
var
  frec: TSQLMyRec;
begin
  fRec:= TSQLMyRec.Create(dbClient, 1, true);
  try
    EditRecord(fRec);
  finally
    dbClient.UnLock(fRec);
  end;
end;

migajek

Member

Registered: 2010-10-01

Posts: 89

Re: Update - memory corruption problem?

thanks, I missed it somehow ... sorry.