Recent

Author Topic: Advanced Records Initialisation bug ?  (Read 1702 times)

Nitorami

  • Sr. Member
  • ****
  • Posts: 431
Advanced Records Initialisation bug ?
« on: August 13, 2022, 10:15:53 am »
Advanced Records allow to define Class operators initialise/finalise to autoinitialise/clean up the record. That is very handy and spares extra calls to Create and Free as for Classes.

I used this for a random generator; the generator is a record and its state is autoinitialised. Works brillant when the record is global, also works when it is a Class field, BUT fails within a Class hierarchy. See below.
The generator rgen is a field of TSys0, and is correctly initilalised when creating an instance of TSys0, but NOT when creating an instance of TSys. Although TSys.Create calls TSys0.Create, rgen is not initialised.... have I misunderstood something ?

Attached the source files for testing. Sorry no Lazarus project, I am still used to work with the Textmode IDE. And I know the generator is crap, this is just for testing.
FPC3.2.2, win32, mode objfpc

Code: Pascal  [Select][+][-]
  1. uses randgen2;
  2.  
  3. type TSys0 = Class
  4.        rgen: TRandgen;
  5.        constructor create;
  6.        destructor destroy; override;
  7.      end;
  8.  
  9. constructor TSys0.Create;
  10. begin
  11.   writeln ('TSys0 now being created');
  12.   Inherited create;
  13.   writeln ('Print random numbers');
  14.   writeln (rgen.random32);
  15.   writeln (rgen.random32);
  16.   writeln (rgen.random32);
  17.   writeln (rgen.random32);
  18. end;
  19.  
  20. destructor TSys0.destroy;
  21. begin
  22.   inherited destroy;
  23. end;
  24.  
  25. //############################
  26.  
  27. type TSys = Class (TSys0)
  28.        constructor create;
  29.        destructor destroy; override;
  30.      end;
  31.  
  32. constructor TSys.Create;
  33. begin
  34.   Inherited create;
  35. end;
  36.  
  37. destructor TSys.destroy;
  38. begin
  39.   inherited destroy;
  40. end;
  41.  
  42. //############################
  43.  
  44. var C: TSys0;
  45.  
  46. begin
  47.   writeln;
  48.   writeln ('Create instance of TSys0...');
  49.   C := TSys0.Create;
  50.   C.Free;
  51.  
  52.   writeln;
  53.   writeln ('Create instance of TSys...');
  54.   C := TSys.Create;
  55.   C.Free;
  56. end.
  57.  

BrunoK

  • Sr. Member
  • ****
  • Posts: 363
  • Retired programmer
Re: Advanced Records Initialisation bug ?
« Reply #1 on: August 13, 2022, 02:53:11 pm »
Code: Pascal  [Select][+][-]
  1. type TSys = Class (TSys0)
  2.        // rgenB: TRandgen; // <-- comment / uncomment changes rgen initialize ~bk
  3.        constructor create;
  4.        destructor destroy; override;
  5.      end;
Modifying TSys fields gives strange result that depend on commenting/un-commenting rgenB.  Strange or did I make a mistake ?

ASerge

  • Hero Member
  • *****
  • Posts: 2022
Re: Advanced Records Initialisation bug ?
« Reply #2 on: August 14, 2022, 12:42:49 am »
Sorry no Lazarus project, I am still used to work with the Textmode IDE. And I know the generator is crap, this is just for testing.
FPC3.2.2, win32, mode objfpc
Bug reproduced. А simplified version that is also suitable for Delphi:
Code: Pascal  [Select][+][-]
  1. {$APPTYPE CONSOLE}
  2. {$IFDEF FPC}
  3.   {$MODE OBJFPC}
  4.   {$MODESWITCH ADVANCEDRECORDS}
  5. {$ENDIF}
  6.  
  7. type
  8.   TState = record
  9.   strict private
  10.     FState: LongWord;
  11.     class operator Initialize({$IFDEF FPC}var{$ELSE}out{$ENDIF} Instance: TState);
  12.   public
  13.     property State: LongWord read FState;
  14.   end;
  15.  
  16. class operator TState.Initialize({$IFDEF FPC}var{$ELSE}out{$ENDIF} Instance: TState);
  17. begin
  18.   Writeln('TState now being initialised');
  19.   Instance.FState := 1;
  20. end;
  21.  
  22. type
  23.   TSys0 = class(TObject)
  24.     FItem: TState;
  25.     constructor Create;
  26.   end;
  27.  
  28.   TSys = class(TSys0)
  29.     //FItem2: TState;
  30.   end;
  31.  
  32. constructor TSys0.Create;
  33. begin
  34.   inherited;
  35.   Writeln(ClassName + ' now being created');
  36.   Writeln('State=', FItem.State);
  37. end;
  38.  
  39. var
  40.   C: TSys;
  41. begin
  42.   Writeln('Create instance of TSys...');
  43.   C := TSys.Create;
  44.   C.Free;
  45.   Readln;
  46. end.
In FPC:
Quote
Create instance of TSys...
TSys now being created
State=0
In Delphi:
Quote
Create instance of TSys...
TState now being initialised
TSys now being created
State=1

PascalDragon

  • Hero Member
  • *****
  • Posts: 4734
  • Compiler Developer
Re: Advanced Records Initialisation bug ?
« Reply #3 on: August 14, 2022, 03:44:56 pm »
Fixed in 27c1bb3b. If no issues occur this will likely be merged to 3.2.3 as well.

Nitorami

  • Sr. Member
  • ****
  • Posts: 431
Re: Advanced Records Initialisation bug ?
« Reply #4 on: October 02, 2022, 04:34:10 pm »
Thanks for the fix.
I reopen this thread because I stumbled across a very similar issue: The same problem occurs when using an instance of an advanced record declared globally in another unit: The initalization is not called.

FPC 3.2.0, win32. Maybe the fix already solved that - sorry I cannot test because I don't understand gitlab.

Code: Pascal  [Select][+][-]
  1. unit testunit;
  2. {$MODE objfpc}
  3. {$MODESWITCH ADVANCEDRECORDS}
  4. INTERFACE
  5. type TGen = record
  6.               a: integer;
  7.               function content: integer;
  8.               class operator Initialize (var aRGen: TGen);
  9.               class operator Finalize   (var aRGen: TGen);
  10.             end;
  11.  
  12. var aGen: TGen;
  13.  
  14. IMPLEMENTATION
  15. function TGen.content: integer;
  16. begin
  17.   result := a;
  18. end;
  19.  
  20. class operator TGen.Initialize (var aRGen: TGen);
  21. begin
  22.   writeln ('init aRGen at address ',ptruint (@aRgen));
  23.   aRGen.a := 123456;
  24. end;
  25.  
  26. class operator TGen.Finalize   (var aRGen: TGen);
  27. begin
  28. end;
  29.  
  30. begin
  31. // initialize (aGen);  //without this, initialize is never called when using aGen from a different unit
  32. end.


Code: Pascal  [Select][+][-]
  1. uses testunit;
  2.  
  3. begin
  4.   writeln (aGen.content);  //prints zero
  5. end.

Arioch

  • Sr. Member
  • ****
  • Posts: 414
Re: Advanced Records Initialisation bug ?
« Reply #5 on: October 02, 2022, 05:54:09 pm »
Maybe the fix already solved that - sorry I cannot test because I don't understand gitlab.

You do not have to.

Provided your internet is fast and disk is large (and that is so for most desktops for last 10 years) you can just use FpcUpDeluxe.
There are caveats though. You have to make many copies of FpcUpDeluxe each manging their own copy of Lazarus.

So what i did recently:

1. I created a folder - d:\Lazarus.Edge\_UpdDx
2. I downloaded the win64 tool (there is win32 tool too) - d:\Lazarus.Edge\_UpdDx\fpcupdeluxe-x86_64-win64.exe
3. In the top left cornet i chnged "Set Install Dir" to D:\Lazarus.Edge
4. In the left "Basic" tab i set - this time - FPC and LAzarus versions to "trunk"  (old SVN term, that is Git is actually "master" or "main")
5. I went into "Setup+" down there to make both FPC and azarus be debug versions - but you probably won't need it. IF you only want to run your tests and see if they fail or succeed, you absolutely don't. But if you want to look into guts of FPC/LAz and hope to identiofy erros and suggest fixes - then you need debug builds.
6. I pressed Install FPC+Laz and went away

One hour later i went back and i had 3,5 GB folder with built FPC and Laz in them. Now i can run d:\Lazarus.Edge\lazarus\startlazarus.exe  and make my tests on bnleedign edhe

However, ii only very rarely do it. My main installation is

d:\fpcupdeluxe\lazarus\
d:\fpcupdeluxe\lazarus_deb\

Those are not "trunk" but "Fixes_2_2: and "Fixes_3_2" instead, they are managed by a different fpcupdeluxe copy too. They also have a number of my custom patches and git branching, etc, that you would not need, yet.

So, make your own folder like d:\Lazarus.stable and populate it like i described abouve, just with different branches (and no debug builds, but you do not need them in both versions anyway)

If to exclude the cloned debug-build nd -rd party libraries (CCR) that folder takes... 5GB. Wow. Guess there is some slack no one clears, maybe i should git gc more often



Arioch

  • Sr. Member
  • ****
  • Posts: 414
Re: Advanced Records Initialisation bug ?
« Reply #6 on: October 02, 2022, 05:58:31 pm »
Fixed in 27c1bb3b. If no issues occur this will likely be merged to 3.2.3 as well.

...and you want to implement BPLs...

One of the most delicious bugs in Delphi XE2 i met were exactly about implicit unit initializations, when the application consists of both statically linked and dynamically loaded BPLs ( plugins scheme );

And the manifestation was const Captions: array [xxx] of string = ( .... ) suddenly becoming empty strings.  I think they fixed it later in XE4 or XE6 but did not test.
It was one big wowser...

KodeZwerg

  • Sr. Member
  • ****
  • Posts: 484
  • Fifty shades of code.
    • Delphi & FreePascal
Re: Advanced Records Initialisation bug ?
« Reply #7 on: October 03, 2022, 01:57:23 am »
The same problem occurs when using an instance of an advanced record declared globally in another unit: The initalization is not called.
From point of logic I would agree that such try needs to fail. It simple would make no sense. Imagine 10 units that include your super-record-unit, each change the value, who is correct?
Initialization should just local happen, this I try to say but I can not say if that is a rule.
« Last Edit: Tomorrow at 31:76:97 by KodeZwerg »

Nitorami

  • Sr. Member
  • ****
  • Posts: 431
Re: Advanced Records Initialisation bug ?
« Reply #8 on: October 03, 2022, 12:32:52 pm »
@Arioch - ok, installed the 3.2 FPC only fixes branch using fpcupdeluxe without problems. As expected, the initial issue is fixed but the issue with global records declared in the interface of another unit not being initialised remains.

@KodeZwerg - my use case may not exactly be what advanced records were designed for but it is legitimate. in the case at hand, the record is a special random number generator. An instance within the interface part of a unit is accessible from everywhere, just as any other variable.
The conflict you presume does not exist; the first call to AGen.random() should initialise the record first and then provide a random number; each subsequent call from wherever will provide another random number, just as for system.random. But instead, initialize is never called and I only get zeros from the generator.
It is easy to get around this by calling initialize(aGen) once in the initalisation part of the unit, but still, the non-working of the automatic initialisation is weird and, I believe, a bug.



PascalDragon

  • Hero Member
  • *****
  • Posts: 4734
  • Compiler Developer
Re: Advanced Records Initialisation bug ?
« Reply #9 on: October 03, 2022, 02:49:37 pm »
Fixed in 27c1bb3b. If no issues occur this will likely be merged to 3.2.3 as well.

...and you want to implement BPLs...

Managed operators were implemented by a third party developer. When one only reviews a feature instead of essentially (re)implementing it (like I have done with anonymous functions) then there are use and test cases that more likely slip ones mind.

Also managed records and dynamic packages are at two different places of the initialization: the issue with managed records is here that the implicit initialization function is not populated correctly, because managed records are the first types that aren't initialized to 0 / Nil, but instead need more functionality. Dynamic packages on the other hand basically need a rework of the whole unit initialization/finalization which includes the implicit initialization as a little part (though how implicit initialization itself works wouldn't be touched as part of this).

I reopen this thread because I stumbled across a very similar issue: The same problem occurs when using an instance of an advanced record declared globally in another unit: The initalization is not called.

Please report a bug.

Thaddy

  • Hero Member
  • *****
  • Posts: 12148
Re: Advanced Records Initialisation bug ?
« Reply #10 on: October 03, 2022, 06:03:54 pm »
And please also check the declaration order in the uses clause.
So if you want a stupid government and not want to live free? Go ahead. Don't count out old tank commanders stupid russian government. With a minor r.... Even my best russians friends are completely finished with such stupidity. My crew and I never missed....

Arioch

  • Sr. Member
  • ****
  • Posts: 414
Re: Advanced Records Initialisation bug ?
« Reply #11 on: October 03, 2022, 09:09:51 pm »
Also managed records and dynamic packages are at two different places of the initialization: the issue with managed records is here that the implicit initialization function is not populated correctly, because managed records are the first types that aren't initialized to 0 / Nil, but instead need more functionality. Dynamic packages on the other hand basically need a rework of the whole unit initialization/finalization which includes the implicit initialization as a little part (though how implicit initialization itself works wouldn't be touched as part of this).

Infeed, running initializers for global vars is kind of the same as running class constructors.

And then, when you mix static BPLs, dynamic BPLs, DLLs - it become a mess.

For example, in some cases XE2 finalizes units that that would be used later again. It was exactly this finalization that killed stringarray consts. And i don't remember if those units were re-initialized later, but if they were then it did not re-establihsh the constants.

This, probably, was moare a bug of const long strings (RefCount < 0) being subjects to finalization, which whould not had happen.
Or not, this is all about concepts.

If you care i might look for my old test project on this issue, so you would have a particularly nasty BPL code-twister for tests.
Probably you can also find it in Delphi Jira, i think i did report it there when it still was QA.

PascalDragon

  • Hero Member
  • *****
  • Posts: 4734
  • Compiler Developer
Re: Advanced Records Initialisation bug ?
« Reply #12 on: October 04, 2022, 01:42:33 pm »
If you care i might look for my old test project on this issue, so you would have a particularly nasty BPL code-twister for tests.

Test cases for that are definitely welcome.

Arioch

  • Sr. Member
  • ****
  • Posts: 414
Re: Advanced Records Initialisation bug ?
« Reply #13 on: October 04, 2022, 10:33:13 pm »
Okay, i'll see.

Yesterday i tested one much simpler XE2 bug, where string constants underwent double-free.
FPC was not affected.
Would attach it for the sake of history, how simple it might get to ruin managed types with absolutely orthodox code.

There was some obscure case in XE FastCode (optimized ASM RTL) that interfered with procedure (const string) wrong way, essentially freeing the constant after the first call, and on the second call running amok.

This bug did NOT reproduce in FPC RTL, neither with AnsiString, nor UnicodeString.
Granted, FPC RTL is verbose and written in Pascal :-)

So, that specific problem i had maybe won't manifest with FPC anyway.

In a sence, it was use-after-free. The unit was finalized, then used again. Unit finalizatino was erroneously freeing items of  "const X: array [xxx] of string". So i believe those bugs were related, and FPC would not have that exact problem.

However, there can be some other unexpected problems with initialize/finalize sequences..

I'll look tomorrow for my dusty corners for that BPL sample.
The attached one is NOT it. But probably different manifestation of same or similar XE2 bug.

Arioch

  • Sr. Member
  • ****
  • Posts: 414
Re: Advanced Records Initialisation bug ?
« Reply #14 on: October 05, 2022, 11:59:38 pm »
Ooook. Found the BPL twister. Not, it would probably not cry foul on FPC, as FPC seem to have correct const longstring finalizer.
But maybe other cases would.

The first question is - which of the units in the package and when should be initialized/finalized?

The secondary question would be WHAT EXACTLY is included in initialization/finalization. Like const longstrings, if they belong to PE/ELF data segment, then they are no subject to be initialized-  but then they neither should be finalized, because if we have multiple init/fini-init-fini-init-fini runs without dll/so unloaded and reloaded - that data ends up destroyed

The tertiary quesiton would be platform restrictions. For example in the Windows DLL Delphi units initialization sections are called withing DLLMain context, in which execution environment was NOT establioshed yet, and 99% of Win32 API are prohibited to be called. AFAIR that is not do with BPLs but it is so with DLLs (DevExpress used to have code that was working in BPLs but not DLLs, and in mixed BPL+monolythic-DLL projects that was a total mess screwing even IDE debugger)

Back to the first question. Imagine we have  A.BPL carrying units A1 to A4, and B.BPL caryrin B1 to B4.

We also have EXE, that is statically linked to A.BPL but not to B.BPL
A.BPL serves, among other things, a patch panel for plugins.
B.BPL is one of those plugins, it gets dynamically loaded (`LoadPackage`) to do some service, then unloaded.

EXE is linked to A1 and A2 units.
B.BPL units are linked to A2 and A3.

How does Delphi XE2 behave? When EXE starts it calls initializers for A1 and A2. It does not call initializers for A3 and A4. This optimization saves time and memory, but complicates things. I am not sure it was worth it.
When EXE runs LoadPackage(B) it does not have (and can not have) compile-time informaiton which B's units would end up used, so it runs initializers for ALL B1 to B4 units. This all-or-some strategies are different.

Now, B.BPL should do something about units A2 (initialized by EXE) and A3 (not initialized). I do not remember exactly what it does.

Then we have UnloadPackage(B), and after B1 to B4 are deinitialized - the question comes if A2 and A3 should be deinitialized.  Maybe Delpihi RTL should have made reference counting, maybe it even did so - i don't remember those details, they looked kind of spagetti to me and i was bad at memorizing them.

Can finalization of ALL units be deferred until the binary file is unloaded (BPL or EXE)?
If developers follow RAII pattern, then surely A3 unit should be finalized by B's unloading. Because who knows which platform resources were locked by class constructors. And if we set on "never finlize until unmapping binary file" - then we can trigger platform resources leak.

However, can we be sure all the developers develop their units in a way, that would not break on multiplt init-fini-init-fini? Or, it would it just be their blame not yours?

I don't know which evil it lesser here.

--------------

There is a yet worse can of worms that i did not explore, because i already had enough, and because i was exploring specific trouble of constants destruction.
When if we have TWO loadable packages, B and C ? And C would use A1,  A3 and A4 units (B used A2 and A3, EXE linked to A1 and A2)

And then we have overlapped B-load, C-load, B-unload, C-unload cycle.

This seems to go to combinatorial explostion, if units initialization and finalization would not become some smart and sealed abstraction (personalyl i think today units should just be classes, conceptually)

--------

So, the attached are sources for this EXE/A/B triad, that makes const static array of longstrings be destoryed on Delphi XE2.  Probably would not do it on FPC, but might serve as some framework for other managed types and for more convoluted use cases (like EXE/A/B/C).

The log, when executed with 0 (no workarounds) mode this project writes the following, attached too (it is sad this forum has no spoilers).



 

TinyPortal © 2005-2018