Initialization & finalization of objects

[LemonParty]

Hello.

Do automatic initialization and finalization works on objects?

I have this fragments of code:

Code: Pascal  [Select][+][-]

1. type
2.   TTimerRec = object
3.   public
4.     ID: SizeUInt;
5.     Name: String[47];
6.     LastUpdated: TDateTime;
7.     Frames: array of TTimerFrame;
8.     function CMDLine: TStringArray;
9.     function TotalDuration: DWord;
10.   end;
11. {...}
12.   FTimerRecs: array of TTimerRec;
13. {...}
14. var
15.   TR: TTimerRec;
16. {...}
17.     TR:= FTimerRecs[sgTimers.Selection.Bottom];
18.     Move(
19.       FTimerRecs[sgTimers.Selection.Top - 1],
20.       FTimerRecs[sgTimers.Selection.Top],
21.       (sgTimers.Selection.Bottom - sgTimers.Selection.Top + 1) * SizeOf(TTimerRec));
22.     FTimerRecs[sgTimers.Selection.Top - 1]:= TR;

So, after executing the last fragment of code some values (strings and dynamic arrays) of TTimerRec turn into garbage.

[Xor-el]

Hello.

Do automatic initialization and finalization works on objects?

I have this fragments of code:

Code: Pascal  [Select][+][-]

1. type
2.   TTimerRec = object
3.   public
4.     ID: SizeUInt;
5.     Name: String[47];
6.     LastUpdated: TDateTime;
7.     Frames: array of TTimerFrame;
8.     function CMDLine: TStringArray;
9.     function TotalDuration: DWord;
10.   end;
11. {...}
12.   FTimerRecs: array of TTimerRec;
13. {...}
14. var
15.   TR: TTimerRec;
16. {...}
17.     TR:= FTimerRecs[sgTimers.Selection.Bottom];
18.     Move(
19.       FTimerRecs[sgTimers.Selection.Top - 1],
20.       FTimerRecs[sgTimers.Selection.Top],
21.       (sgTimers.Selection.Bottom - sgTimers.Selection.Top + 1) * SizeOf(TTimerRec));
22.     FTimerRecs[sgTimers.Selection.Top - 1]:= TR;

So, after executing the last fragment of code some values (strings and dynamic arrays) of TTimerRec turn into garbage.

I reported similar bugs (but for records) around 2018. I checked again a few weeks back and was quite surprised to see that one of them is still open and unresolved.

https://gitlab.com/freepascal.org/fpc/source/-/work_items/33006
https://gitlab.com/freepascal.org/fpc/source/-/work_items/33205

You might want to report your issue on GitLab as well and reference the ones I linked above.

[Martin_fr]

Arrays (dynamic) and strings (ansi/long) do work in objects, just like they do in records.

But your problem is

Code: Pascal  [Select][+][-]

1.  Move(
2.       FTimerRecs[sgTimers.Selection.Top - 1],
3.       FTimerRecs[sgTimers.Selection.Top],
4.       (sgTimers.Selection.Bottom - sgTimers.Selection.Top + 1) * SizeOf(TTimerRec));

Move does not work with managed data. It does not matter if you do

move (MyString, MyOtherString, sizeof(pointer))

Or if the string/array is in a record/object and you move that.

You
- throw away the internal pointer of the string/array  on the target object => but you do not decrease the refcount
- duplicate the internal pointer of the string/array of the source => but you do not increase the ref count

So, if the string/array was NOT empty, then you
- leaked one  string/array
- corrupted another (which may later cause a crash, if unlucky)


If you do the above with classes, and have an array of TMyObject, then you copy the pointer to TMyObjcet, and that is not refcounted. The data in the instance would not be moved, and strings/arrays would not be corrupted.

[LemonParty]

Martin_fr, are you sure that this is wrong? The thing that I acheive with move – I move 1 or more elements of FTimerRecs, so indexes of this elemets increases by 1 and 1 element (TR) being swaped. This operation do not increase or decrease total amount of elements in array, in theory it should not corrupt dynamic arrays.

[Martin_fr]

Martin_fr, are you sure that this is wrong? The thing that I acheive with move – I move 1 or more elements of FTimerRecs, so indexes of this elemets increases by 1 and 1 element (TR) being swaped. This operation do not increase or decrease total amount of elements in array, in theory it should not corrupt dynamic arrays.

Well the single move you have is a problem..

If you swap or rotate, you need one "temp slot" => and that must be cleared before and after. And clearing after is not assigning default/empty. It involves assigning to a pointer cast.

You should have some code looking like

Code: Pascal  [Select][+][-]

1.  Pointer( TempObject.Frames ) := nil;

If you don't have that, then you most likely cause a problem by using move.
(or alternatively you may be using fillchar)


Also, mind you, official statement AFAIK is: you can't use move involving managed data. period.

Truth is, you can. But you rely on internal behaviour of the existing compiler version(s). It will likely work with new ones too, but that is not guaranteed.

[LemonParty]

I guess I solved this problem. I added

Code: Pascal  [Select][+][-]

1. FillChar(TR, SizeOf(TR), 0);

So now code look like:

Code: Pascal  [Select][+][-]

1.     TR:= FTimerRecs[sgTimers.Selection.Bottom];
2.     Move(
3.       FTimerRecs[sgTimers.Selection.Top - 1],
4.       FTimerRecs[sgTimers.Selection.Top],
5.       (sgTimers.Selection.Bottom - sgTimers.Selection.Top + 1) * SizeOf(TTimerRec));
6.     FTimerRecs[sgTimers.Selection.Top - 1]:= TR;
7.     FillChar(TR, SizeOf(TR), 0);

I think TR is managed automatically and compiler tries to release it after completion of procedure which is not needed.

[Martin_fr]

I think TR is managed automatically and compiler tries to release it after completion of procedure which is not needed.

Yes, the temp is freed by the compiler if it still has a "moved" copy, and that is a double free.

Technically, as a local var (?) it is also not initialized, but as a local var with managed data, in the current implementation it is.

The general rule for move is

1) make sure the target area (that will be overriden, and is not overlap with the source) is empty => this is by setlength or :=''
2) move
3) fillchar any source part that wasn't written by overlapped targed

In your case that may just be 3, and that may just be the temp var.

[LemonParty]

I have done further testing and detected that problem still present.
Than I decided to do more radical actions. The result is this code:

Code: Pascal  [Select][+][-]

1. var
2.   Tmp: array [0..SizeOf(TTimerRec)-1] of Byte;
3. {...}
4.     Move(FTimerRecs[sgTimers.Selection.Bottom], Tmp, SizeOf(Tmp));
5.     Move(
6.       FTimerRecs[sgTimers.Selection.Top - 1],
7.       FTimerRecs[sgTimers.Selection.Top],
8.       (sgTimers.Selection.Bottom - sgTimers.Selection.Top + 1) * SizeOf(TTimerRec));
9.     Move(Tmp, FTimerRecs[sgTimers.Selection.Top - 1], SizeOf(Tmp));

Working normally.

[Martin_fr]

Ah, yes: array of byte. Clever. Looks good.

[runewalsh]

There shouldn’t be any “problems that require testing”, you just need to understand the topic once. Try this:

Code: Pascal  [Select][+][-]

1. {$mode objfpc} {$longstrings on}
2. uses
3.         SysUtils;
4. var
5.         a: array[0 .. 1] of string;
6.         temp: string;
7. begin
8.         writeln('Ver. 1');
9.         WriteStr(a[0], 'First string');
10.         WriteStr(a[1], 'Second string');
11.         writeln('String reference counts: ', StringRefCount(a[0]), ', ', StringRefCount(a[1]));
12.         temp := a[0];
13.         writeln('String reference counts after temp := a[0] (correct): ', StringRefCount(a[0]), ', ', StringRefCount(a[1]));
14.         writeln('...temp is finalized...');
15.         Finalize(temp); // also done automatically at ‘end’.
16.         writeln('String reference counts after Finalize(temp) (correct): ', StringRefCount(a[0]), ', ', StringRefCount(a[1]));
17.         writeln('a[0] = ', a[0]);
18.  
19.         writeln(LineEnding, 'Ver. 2');
20.         WriteStr(a[0], 'First string');
21.         WriteStr(a[1], 'Second string');
22.         writeln('String reference counts: ', StringRefCount(a[0]), ', ', StringRefCount(a[1]));
23.         Move(a[0], temp, sizeof(temp)); // Simpler and more efficient alternative for pointer-sized types like strings: pointer(temp) := pointer(a[0]).
24.         writeln('String reference counts after Move(a[0], temp) (wrong): ', StringRefCount(a[0]), ', ', StringRefCount(a[1]));
25.         writeln('...temp is finalized...');
26.         Finalize(temp);
27.         writeln('String reference counts after Finalize(temp) (wrong): ', StringRefCount(a[0]), ', ', StringRefCount(a[1]));
28.         writeln('going to write a[0], beware...');
29.         readln;
30.         writeln('a[0] = ', a[0]);
31.         readln;
32. end.

Move ignores managing, so you should either use := or handle the managing manually.

Ideally, the program should explicitly crash on freeing a string twice, but, uhm... that would be a bit tricky and require more checks in ...decr_ref, which means punishing good code that uses := in favor of bad code that uses Move without complete understanding of what’s going on, thus was asking for trouble from the start — if not this, then something else.