Commit bf116a0 leads to IPO in Delphi 12

sakura · 2024-03-26 07:56:03

Hi,

this change leads to an IPO in Delphi 12:
https://github.com/synopse/mORMot2/comm … e4356bc106

If I followed correctly, this leads Delphi to release the same memory twice.

I assign the text generated by GetAsText to a specific entry of a string array, and when that array is freed later on, I get the following error:

    "EInvalidPointer": {
      "ClassName": "EInvalidPointer",
      "Address": "7f0972d0",
      "Message": "Invalid pointer operation"
    }

Reverting that code into the previous version, everything works fine.

Regards,
Daniel

ab · 2024-03-26 09:57:07

Weird, I was not able to reproduce it with FPC.
It may depend how the result RawUtf8 is consumed...

Anyway, I will revert it because it seems to be unneeded and unsafe - premature optimization or over optimization.
Which is the root of all evil, said.
https://github.com/synopse/mORMot2/commit/7c4aad8b

sakura · 2024-03-26 10:52:59

Thanks, will check asap.

tbo · 2024-03-26 13:50:14

ab wrote:

Anyway, I will revert it because it seems to be unneeded and unsafe - premature optimization or over optimization.
Which is the root of all evil, said.

Are you sure that's what you wanted?

 
else if (StartPos = 0) and
        (Len = L) and
        (PStrCnt(PAnsiChar(pointer(fDataString)) - _STRCNT)^ = 1) then
  FastAssignUtf8(Text, fDataString) // fast return the fDataString instance
else

The description says: "fast return the fDataString instance", but the second part of the sentence says: "and set src to Nil". After calling function FastAssignUtf8, "fDataString" is empty. The function should then better called: GetAsTextAndEmptyStreamIfLenIsSize().

With best regards
Thomas

Last edited by tbo (2024-03-26 13:53:18)

ab · 2024-03-26 15:20:44

This DataString is an internal implementation detail and should not be used directly.
It is documented as "low level", and should not be used outside of the framework.

Let GetAsText() return the DataString instance is a huge and welcome optimization, because this DataString will never be used afterwards, and it avoid allocating and copying this RawUtf8 instance.
So it is really worth it. And since DataString is left to '' it won't make any GPF.

I have made this explicit in the doc:
https://github.com/synopse/mORMot2/commit/a8e713da

sakura · 2024-03-28 08:02:18

Sorry, couldn't check earlier, but it works as before again. Thanks.

mORMot Open Source

#1 2024-03-26 07:56:03

Commit bf116a0 leads to IPO in Delphi 12

#2 2024-03-26 09:57:07

Re: Commit bf116a0 leads to IPO in Delphi 12

#3 2024-03-26 10:52:59

Re: Commit bf116a0 leads to IPO in Delphi 12

#4 2024-03-26 13:50:14

Re: Commit bf116a0 leads to IPO in Delphi 12

#5 2024-03-26 15:20:44

Re: Commit bf116a0 leads to IPO in Delphi 12

#6 2024-03-28 08:02:18

Re: Commit bf116a0 leads to IPO in Delphi 12

Board footer