issue with _Safe and InitArrayFromCsvFile

danielkuettner · 2024-10-02 10:00:44

Hi ab,

the using of InitArrayFromCsvFile in this context overrides memory and results in unexpected behavior:

_Safe(aVariant)^.InitArrayFromCsvFile(aRawUtf8data, [], ';', '"');

But when using a TDocVariantData I can use InitArrayFromCsvFile without issues:

aDocVariantData.InitArrayFromCsvFile(aRawUtf8data, [], ';', '"');

Do you have what's the background of that?

ab · 2024-10-02 14:39:08

Writing

_Safe(aVariant)^.Init....()

just does not make any sense.

danielkuettner · 2024-10-02 18:00:02

Can you explain a little bit. I don't know why it does make any sense.

_Safe(aVariant) gives a TDocVariantData record back and this type has the method InitArrayFromCsvFile.

ab · 2024-10-02 18:19:54

This is not because the method is available that you could call it.
For instance, you should not call Create() over an existing object, or Destroy() several times.

_Safe() returns :
- either an existing TDocVariantData which should not be re-initialized directly via any Init...() command without a previous Clear.
- either a global "fake" TDocVariant constant, which should be read-only by definition.

danielkuettner · 2024-10-02 18:31:25

Ok, but what’s wrong with a re-initialize of a variant? I could re-initialize it who often I want?
It’s only one variant on the stack and not a class-instance of a create constructor.
I create a Variant with _ARR and will fill this Variant with the data of a csv.
Is the using of _Safe(of a Variant of type 271) in this case so clearly senseless?

tbo · 2024-10-02 19:01:39

danielkuettner wrote:

Can you explain a little bit.

Maybe this article will help you.

With best regards
Thomas

ab · 2024-10-02 19:44:04

A TDocVariantData is not a variant, it is a record with managed fields, which could be stored within a variant.
Init*() are to be done on a variant / TVarData instance.
If you call them several times on the same TDocVariantData, you need to call Clear in between.
When you use _Safe() you return a PDocVariantData which is a pointer.

This is confusing, but
- I don't see any other implementation scheme;
- this is the tradeoff of being able to map a variant with a TDocVariantData;
- this is the reason why we made IDocList and IDocDict, which have a very clear behavior, and about their instance life time.

If you are concerned or confused with TDocVariant, either
- switch to IDocList and IDocDict;
- or only use TDocVariantData, with a Clear method call before any Init*().

danielkuettner · 2024-10-02 20:05:57

Ok, @tbo thanks for your good and detailed article.
@ab thanks for your outstanding framework. I'm fare from any critic at all.

But I just can't find any forbidden at this (also after reading your article @tbo):

v:= _ARR([]);
_Safe(v)^.InitArrayFromCsvFile('f1,f2,f3'#10'"1";"2";"3"', [], ',','"');

It seems to work, but am I wrong?

How should I call clear here? What would clear do with an empty []?

I just ask to understand and learn, not to annoy.

Last edited by danielkuettner (2024-10-02 20:14:07)

tbo · 2024-10-02 20:51:25

danielkuettner wrote:

How should I call clear here? What would clear do with an empty []?

v.Clear;
TDocVariantData(v).Clear;

Set breakpoints as described in article under point 3. Then you can see what is going on.

With best regards
Thomas

danielkuettner · 2024-10-03 06:04:52

A clear after each Init is necessary because Variants are managed types, right?
Otherwise the compiler would free a Variant which may be still in using because of a duplicate Init

ab · 2024-10-03 06:21:06

The problem of Init is that it resets its memory, when applied to a variant instance:

procedure TDocVariantData.Init(const aOptions: TDocVariantOptions);
begin
..
  pointer(VName)  := nil; // to avoid GPF when mapped within a TVarData/variant
  pointer(VValue) := nil;

So

v:= _ARR([]);
_Safe(v)^.InitArrayFromCsvFile('f1,f2,f3'#10'"1";"2";"3"', [], ',','"');

works by chance, because those pointers are still void.

But

v:= _ARR([1, 2, 3]);
_Safe(v)^.InitArrayFromCsvFile('f1,f2,f3'#10'"1";"2";"3"', [], ',','"');

will leak memory, since VValue[] = [1,2,3] won't be properly released but its pointer reset to nil.
Just like the following would leak memory,:

o := TObject.Create; 
writeln(o.ClassName);
o := TObject.Create;

danielkuettner · 2024-10-03 06:53:49

Thanks for explaining and your patience!

ab · 2024-10-03 07:02:48

Note that

v:= _ARR([1, 2, 3]);
_Safe(v)^.Clear;
_Safe(v)^.InitArrayFromCsvFile('f1,f2,f3'#10'"1";"2";"3"', [], ',','"');

won't leak any memory.

You are not the only one confused with this "variant-record-wrapper" thing.
This is the main reason why we made IDocList and IDocDict.

The only possibility could be to hide this TDocVariantData... or at least hide its Init() methods and replace them with factory functions...
But it would break a lot of existing code now...

danielkuettner · 2024-10-03 07:11:19

I wouldn't change so much. My confusing results from working with your tools and not understanding much of it in depth. Some points you understand after years.

But perhaps some small improvements could help.
That _Safe could return a read only fake DocVariantData is perhaps a little bit too much and contrary to the name „_safe“.

Last edited by danielkuettner (2024-10-03 07:21:37)

ab · 2024-10-03 07:18:30

I tried to make it more explicit in the doc:
https://github.com/synopse/mORMot2/commit/3db3dfcd

Also explained in TDocVariantData.Clear why there would be some memory leak.

tbo · 2024-10-03 19:45:17

danielkuettner wrote:

That _Safe could return a read only fake DocVariantData is perhaps a little bit too much and contrary to the name „_safe“.

No. That is why you can write:

var
  v: Variant;
  isPeaPuree: Boolean;
begin
  isPeaPuree := _Safe(v).O_['One'].O_['Two'].O_['Three'].B['IsPeaPuree'];
  ShowMessage(IsPeaPuree.ToString(TUseBoolStrs.True));

You can easily reproduce the problem with the Init function. Switch on ReportMemoryLeaksOnShutdown. Write the following:

var
  names: TRawUtf8DynArray;
begin
  names := ['one', 'two'];
  names := Nil;

Set a Breakpoint at names := Nil. When you have reached this point during debugging, press Ctrl+Alt+D and open the disassembly window. Now you can see that the compiler has automatically inserted a call to DynArrayClear for this Nil assignment. However, if you write Pointer(names) := Nil, this does not happen and the dynamic array can no longer be cleaned up at the end. In this case, FastMM will report the lost memory.

With best regards
Thomas

mORMot Open Source

#1 2024-10-02 10:00:44

issue with _Safe and InitArrayFromCsvFile

#2 2024-10-02 14:39:08

Re: issue with _Safe and InitArrayFromCsvFile

#3 2024-10-02 18:00:02

Re: issue with _Safe and InitArrayFromCsvFile

#4 2024-10-02 18:19:54

Re: issue with _Safe and InitArrayFromCsvFile

#5 2024-10-02 18:31:25

Re: issue with _Safe and InitArrayFromCsvFile

#6 2024-10-02 19:01:39

Re: issue with _Safe and InitArrayFromCsvFile

#7 2024-10-02 19:44:04

Re: issue with _Safe and InitArrayFromCsvFile

#8 2024-10-02 20:05:57

Re: issue with _Safe and InitArrayFromCsvFile

#9 2024-10-02 20:51:25

Re: issue with _Safe and InitArrayFromCsvFile

#10 2024-10-03 06:04:52

Re: issue with _Safe and InitArrayFromCsvFile

#11 2024-10-03 06:21:06

Re: issue with _Safe and InitArrayFromCsvFile

#12 2024-10-03 06:53:49

Re: issue with _Safe and InitArrayFromCsvFile

#13 2024-10-03 07:02:48

Re: issue with _Safe and InitArrayFromCsvFile

#14 2024-10-03 07:11:19

Re: issue with _Safe and InitArrayFromCsvFile

#15 2024-10-03 07:18:30

Re: issue with _Safe and InitArrayFromCsvFile

#16 2024-10-03 19:45:17

Re: issue with _Safe and InitArrayFromCsvFile

Board footer