Recent

Author Topic: Smart pointers revisited. Let me know what you think.  (Read 11321 times)

y.ivanov

  • Sr. Member
  • ****
  • Posts: 338
Re: Smart pointers revisited. Let me know what you think.
« Reply #30 on: January 24, 2022, 05:45:52 pm »
My apologies for the necroposting!

Recently, I've decided to take advantage of that smart pointers and I found there is an important operator missing, AddRef.
Cosidering the following usage as ByVal parameter:

Code: Pascal  [Select][+][-]
  1. type
  2.   TAutoTest = specialize TAuto<TTest>;
  3. var
  4.   GlobRef: TAutoTest;
  5.  
  6. procedure A(R: TAutoTest);
  7. begin
  8.   R.Instance.Value := -2;
  9. end;
  10.  
  11. begin
  12.   GlobRef.Instance := TTest.Create(-1);
  13.   A(GlobRef);
  14.   GlobRef.Instance.Value := -3; // SIGSEGV!
  15. end.
  16.  

It needs an AddRef to increase the reference count, like:
Code: Pascal  [Select][+][-]
  1. type
  2.   generic TAuto<T: class> = record
  3.    // ...
  4.   strict private
  5.    //...
  6.     class operator AddRef(var Self: TAuto); inline;
  7.    //...
  8.   end;
  9.  
  10. //...
  11.  
  12. class operator TAuto.AddRef(var Self: TAuto);
  13. begin
  14.   if Assigned(Self.FRef) then
  15.     InterlockedIncrement(Self.FRef^.RefCount);
  16. end;  

Thaddy

  • Hero Member
  • *****
  • Posts: 11517
Re: Smart pointers revisited. Let me know what you think.
« Reply #31 on: January 24, 2022, 06:18:58 pm »
Do you use {$interfaces corba} by any change? I have never seen the need for that in my TAuto<>. Can you confirm you are using ,y version? and COM? Or are you using any other suggestion from the discussion? In that case I can not help you. My version works different from the code you use, that's why I ask. May be you used the version with management operators? That has issues indeed.
« Last Edit: January 24, 2022, 06:26:37 pm by Thaddy »
Путин преступник. Россияне дезинформированы.

Warfley

  • Hero Member
  • *****
  • Posts: 814
Re: Smart pointers revisited. Let me know what you think.
« Reply #32 on: January 24, 2022, 06:32:20 pm »
Note that as long as the double finalize bug is not fixed in the compiler, management  operators do not work if they rely on Finalize.

Therefore if you want to use smartpointers you need to use COM interfaces for now.

My bug report to the double finalize issue:
https://gitlab.com/freepascal.org/fpc/source/-/issues/37164
I think I saw another bug report but the search  in gitlab is terrible. But basically if a function returns a managed  type, it will be finalized twice. As this relies on implicit casts, which are just hidden function calls, these are impacted by this. So if you use the implicit cast to create the smart pointer, it will double finalize and result in dangling pointers
« Last Edit: January 24, 2022, 06:35:06 pm by Warfley »

y.ivanov

  • Sr. Member
  • ****
  • Posts: 338
Re: Smart pointers revisited. Let me know what you think.
« Reply #33 on: January 24, 2022, 07:06:01 pm »
Do you use {$interfaces corba} by any change? I have never seen the need for that in my TAuto<>. Can you confirm you are using ,y version? and COM? Or are you using any other suggestion from the discussion? In that case I can not help you. My version works different from the code you use, that's why I ask. May be you used the version with management operators? That has issues indeed.
My observations are based on code from this discussion (for TAuto<>, not for TGSharedRef<>). I'm not using {$interfaces corba}.
But because of Warfley post, may be I should double-check with other compiler versions, mine is not recent.

Thaddy

  • Hero Member
  • *****
  • Posts: 11517
Re: Smart pointers revisited. Let me know what you think.
« Reply #34 on: January 24, 2022, 07:15:55 pm »
As Warfley observed, the original version( by me) based on an inteface does not suffer the issue.
It is also easier to use because of auto-create (see Tstringlist example)
Путин преступник. Россияне дезинформированы.

y.ivanov

  • Sr. Member
  • ****
  • Posts: 338
Re: Smart pointers revisited. Let me know what you think.
« Reply #35 on: January 25, 2022, 01:55:18 am »
As Warfley observed, the original version( by me) based on an inteface does not suffer the issue.
It is also easier to use because of auto-create (see Tstringlist example)
Correct, I can see into the compiler sources that the interfaces are handled quite differently.

I'm currently playing with the TAuto<> class from https://forum.lazarus.freepascal.org/index.php/topic,46306.msg330408.html#msg330408
 as they have a reference counter which is of my particular interest. My post was about that generics (I believe yours is named Auto<>).

@Warfley
Although not 100% sure, the issue with the double finalization (in https://gitlab.com/freepascal.org/fpc/source/-/issues/37164) is related to the creation of the hidden temp variable for the enumerator. So with the simplified example given by Serge Anvarov (ASerge?) for the same issue - when the function result is assigned to a variable, the issue dissapears, when discarded - we have double finalization.

So far, I don't see how the ASerge TAuto<> can be affected by the double finalization issue. Still investigating.



Thaddy

  • Hero Member
  • *****
  • Posts: 11517
Re: Smart pointers revisited. Let me know what you think.
« Reply #36 on: January 25, 2022, 07:17:20 am »
You can get at the refcount from the IInterface by a cast to TInterfacedobject which is the implementing class and has a refcount. I will see if I can surface that..
Add this to the interface based Auto<T>:
Code: Pascal  [Select][+][-]
  1.    function Auto<T>.RefCount:integer;
  2.    begin
  3.      Result := 0;
  4.      if Assigned(FFreeTheValue) then
  5.        Result := (FFreeTheValue as TInterfacedObject).RefCount;
  6.    end;
Untested! Somewhat tested. Seems to work.
Now your specialization of Auto<T> has  a refcount...
Attached full unit  + demo

Again, this is ONLY for the initial interface based Auto<T> by me.
« Last Edit: January 25, 2022, 12:32:40 pm by Thaddy »
Путин преступник. Россияне дезинформированы.

Thaddy

  • Hero Member
  • *****
  • Posts: 11517
Re: Smart pointers revisited. Let me know what you think.
« Reply #37 on: January 25, 2022, 10:12:49 am »
Note, that with hindsight I kept the original [box/unbox] syntax, because that allows for explicit release, see the attached example above. My later changes where merely to satisfy customers here..  :D
Путин преступник. Россияне дезинформированы.

y.ivanov

  • Sr. Member
  • ****
  • Posts: 338
Re: Smart pointers revisited. Let me know what you think.
« Reply #38 on: January 25, 2022, 10:56:15 am »
Note, that with hindsight I kept the original [box/unbox] syntax, because that allows for explicit release, see the attached example above. My later changes where merely to satisfy customers here..  :D
Thanks for the effort!
 
My preferred choice were the advanced records and I presumed all managed objects were treated in the same way by the compiler, too bad it is not the case.
In some other PL, thanks to the templates, multiple inheritance and auto objects, I'm using refcounted objects extensively and that makes my life much easier.

BTW, It is not that the same functionality can't be achieved with TCustomVariantType for example, which in turn is also IInterface by itself. But involving COM and/or Variants sounds like unneeded dead weight to me.

Thaddy

  • Hero Member
  • *****
  • Posts: 11517
Re: Smart pointers revisited. Let me know what you think.
« Reply #39 on: January 25, 2022, 11:16:49 am »
This particular use of COM interfaces is very lightweight. Everything is solved at compile time, specialization is at type level.
The other option with management operators needs to have the double finalize bug fixed and ideally also default properties for other properties than arrays - also on my wishlist -. The burden would be about the same as using interfaces, in my opinion, after examining the generated assembler.
« Last Edit: January 25, 2022, 12:20:27 pm by Thaddy »
Путин преступник. Россияне дезинформированы.

Warfley

  • Hero Member
  • *****
  • Posts: 814
Re: Smart pointers revisited. Let me know what you think.
« Reply #40 on: January 25, 2022, 11:35:38 am »
@Warfley
Although not 100% sure, the issue with the double finalization (in https://gitlab.com/freepascal.org/fpc/source/-/issues/37164) is related to the creation of the hidden temp variable for the enumerator. So with the simplified example given by Serge Anvarov (ASerge?) for the same issue - when the function result is assigned to a variable, the issue dissapears, when discarded - we have double finalization.

So far, I don't see how the ASerge TAuto<> can be affected by the double finalization issue. Still investigating.
The interesting bit about the example by Serge is that no copy operation takes place. If we use a constructor also the assignment fails:
Code: Pascal  [Select][+][-]
  1. program Project1;
  2.  
  3. {$mode objfpc}{$H+}
  4. {$ModeSwitch advancedrecords}
  5.  
  6. type
  7.  
  8.   { TSomeManagedRecord }
  9.  
  10.   TSomeManagedRecord = record
  11.     id: Integer;
  12.     class operator Initialize(var e: TSomeManagedRecord);
  13.     class operator Finalize(var e: TSomeManagedRecord);
  14.     class operator AddRef(var e: TSomeManagedRecord);
  15.     class operator Copy(constref s: TSomeManagedRecord; var d: TSomeManagedRecord);
  16.  
  17.     constructor Create(A: Integer);
  18.   end;
  19. var
  20.   nextId: Integer;
  21.  
  22. class operator TSomeManagedRecord.Initialize(var e: TSomeManagedRecord);
  23. begin
  24.   WriteLn('Initialize: ', nextId);
  25.   e.id:=nextId;
  26.   Inc(nextId);
  27. end;
  28.  
  29. class operator TSomeManagedRecord.Finalize(var e: TSomeManagedRecord);
  30. begin
  31.   WriteLn('Finalize: ', e.id);
  32. end;
  33.  
  34. class operator TSomeManagedRecord.AddRef(var e: TSomeManagedRecord);
  35. begin
  36.   WriteLn('AddRef: ', e.id);
  37. end;
  38.  
  39. class operator TSomeManagedRecord.Copy(constref s: TSomeManagedRecord;
  40.   var d: TSomeManagedRecord);
  41. begin
  42.   WriteLn('Copy: ', s.id, '->',d.id);
  43. end;
  44.  
  45. constructor TSomeManagedRecord.Create(A: Integer);
  46. begin
  47.  
  48. end;
  49.  
  50. procedure Test;
  51. var
  52.   R: TSomeManagedRecord;
  53. begin
  54.   R := TSomeManagedRecord.Create(42);
  55. end;
  56.  
  57. begin
  58.   Test;
  59.   Readln;
  60. end.
Result:
Code: Pascal  [Select][+][-]
  1. Initialize: 0
  2. Initialize: 1
  3. Finalize: 1
  4. Copy: 1->0
  5. Finalize: 0
  6. Finalize: 1
So I guess it's due to optimizations (probably RTO) which happens here that the copy is ommited, and that this optimization is not happening when the constructor is used (I already found out that constructors are weird with respect to optimization, e.g. they are never inlined).
So I think the reason why Serges example does not fail is simply because it is so simple that the optimiser circumvents the bug.

Thaddy

  • Hero Member
  • *****
  • Posts: 11517
Re: Smart pointers revisited. Let me know what you think.
« Reply #41 on: January 25, 2022, 12:10:28 pm »
@Warfley,  I am a bit confused here:
1. that example does not actually assigns something useful, like a stringlist.
2. the copy operator is empty, of course there is no copy. In my example the same: only the refcount is increased, since it is a reference.
3. the create is empty too

I have no clue if ASerge's example that you cite is viable, since it seems not complete, whereas my example is a complete example that always works on any class with a default, parameterless, constructor.
I suppose - as discussed and I acknowledged - that eventually this may very well be the way to go, but as it stands my interface approach has its own merits. It is even Delphi compatible.

Maybe ASerge will expand on his example? It would be interesting to see how that works in its entirity.
It is always good to see if any of us has made any progress on the subject: smart pointers can be very useful, e.g. in object persistent frameworks.
I have no particular affinity to any of both options, as long as they work.. :D
« Last Edit: January 25, 2022, 12:28:17 pm by Thaddy »
Путин преступник. Россияне дезинформированы.

Warfley

  • Hero Member
  • *****
  • Posts: 814
Re: Smart pointers revisited. Let me know what you think.
« Reply #42 on: January 25, 2022, 12:32:17 pm »
I mean serges example to the bug report, not his example in this post, which was also the one  referenced in the quote I replied to (It's confusing I know :D). This is just the minimal example to show the double finalize bug
« Last Edit: January 25, 2022, 12:33:55 pm by Warfley »

y.ivanov

  • Sr. Member
  • ****
  • Posts: 338
Re: Smart pointers revisited. Let me know what you think.
« Reply #43 on: January 25, 2022, 12:34:39 pm »
*snip*
I have no clue if ASerge's example that you cite is viable, since it seems not complete, whereas my example is a complete example that always works on any class with a default, parameterless, constructor.
I suppose - as discussed and I acknowledged - that eventually this may very well be the way to go, but as it stands my interface approach has its own merits. It is even Delphi compatible.
*snip
It is actually about two different examples given by ASerge - one is here for TAuto<> and another one is for the double finalization bug issue : https://gitlab.com/freepascal.org/fpc/source/uploads/43c46a9afc8394afd23b490a50216aec/project1.lpr
Warfley talks for the latter one in his latest post.

@Warfley
I'm still for the hidden temporary variables,
Code: Pascal  [Select][+][-]
  1. procedure Test1;
  2. var
  3.   R: TSomeManagedRecord;
  4. begin
  5.   R := TSomeManagedRecord.Create(42);
  6. end;
  7.  
  8. procedure Test2;
  9. var
  10.   R, S: TSomeManagedRecord;
  11. begin
  12.   R := S.Create(42);
  13. end;
Test2 doesn't suffer from the issue. Test1 involves a temporary.

In both cases fpc_copy_proc is not omitted, just fpc_finalize was erroneously called before Create in Test1.
« Last Edit: January 25, 2022, 12:43:00 pm by y.ivanov »

Thaddy

  • Hero Member
  • *****
  • Posts: 11517
Re: Smart pointers revisited. Let me know what you think.
« Reply #44 on: January 25, 2022, 12:34:53 pm »
Ah, OK. I did not grasp that.

Btw, I attached a renewed version of the example code (auto_refcount.pas) attachment, not the unit. Very small change to explain better, merely comments.
« Last Edit: January 25, 2022, 12:39:53 pm by Thaddy »
Путин преступник. Россияне дезинформированы.

 

TinyPortal © 2005-2018