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

sakura
Member
From: Germany
Registered: 2018-02-21
Posts: 238
Website

Commit bf116a0 leads to IPO in Delphi 12

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

Offline

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

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,550
Website

Re: Commit bf116a0 leads to IPO in Delphi 12

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. wink
https://github.com/synopse/mORMot2/commit/7c4aad8b

Offline

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

sakura
Member
From: Germany
Registered: 2018-02-21
Posts: 238
Website

Re: Commit bf116a0 leads to IPO in Delphi 12

Thanks, will check asap.

Offline

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

tbo
Member
Registered: 2015-04-20
Posts: 349

Re: Commit bf116a0 leads to IPO in Delphi 12

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. wink

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(). big_smile

With best regards
Thomas

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

Offline

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

ab
Administrator
From: France
Registered: 2010-06-21
Posts: 14,550
Website

Re: Commit bf116a0 leads to IPO in Delphi 12

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

Offline

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

sakura
Member
From: Germany
Registered: 2018-02-21
Posts: 238
Website

Re: Commit bf116a0 leads to IPO in Delphi 12

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

Offline

Board footer

Powered by FluxBB