Recent

Author Topic: Is This A Leak?  (Read 9708 times)

del

  • Sr. Member
  • ****
  • Posts: 258
Is This A Leak?
« on: October 14, 2019, 11:38:32 pm »
In principle, in theory, is this a leak?

Code: Pascal  [Select][+][-]
  1. //\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\//
  2.  
  3. class function MTextEd.GetVal(te: TEdit; regPath: string): string;
  4. begin
  5.         WriteToRegistry(te.Text, Concat('SOFTWARE\CustomOps', regPath));
  6.         GetVal := te.Text;
  7. end;
  8.  
  9. //\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\//
  10.  

The purpose of this function is to grab the value from a TEdit control on a form and return that value as a "string" object. It also updates the "registry" (xml file on Linux). Is a "temporary" string object being constructed during this process and falling out of scope but NOT getting freed (destroyed)? In C++ you don't have to worry about this - the destructor is automatically called when the object goes out of scope.

But I need to change my mindset for Pascal. I can always just return a pointer (or do the "var" parameter thing), but that seems a little old school. And the pointer will dangle when the form gets destroyed. I'm assuming that returning booleans, integers, and other primitive types is OK. But "string" is not primitive. Will it get freed by some other means like ref counting?

jamie

  • Hero Member
  • *****
  • Posts: 6090
Re: Is This A Leak?
« Reply #1 on: October 15, 2019, 01:50:14 am »
That is a managed type, it should work.
The only true wisdom is knowing you know nothing

del

  • Sr. Member
  • ****
  • Posts: 258
Re: Is This A Leak?
« Reply #2 on: October 15, 2019, 05:30:26 am »
Cool. Thanks!

Thaddy

  • Hero Member
  • *****
  • Posts: 14205
  • Probably until I exterminate Putin.
Re: Is This A Leak?
« Reply #3 on: October 15, 2019, 06:50:56 am »
Well, no, not cool, at least not optimal:
Both the Tedit and the string are managed types, indeed, but:
The parameters should be passed as const to prevent the reference count to increase:
Code: Pascal  [Select][+][-]
  1. class function MTextEd.GetVal(const te: TEdit; const regPath: string): string;
  2. begin
  3.   WriteToRegistry(te.Text, Concat('SOFTWARE\CustomOps', regPath));
  4.   Result := te.Text; // Result is available. Use it!  instead of name may avoid ambiguity later
  5. end;
Without the const modifier you can - not always - indeed create memory leaks if you are not careful.
It  prevents copy on write semantics, which causes the refcount to go up, so it is safer and much faster code.
Without const/constref or var you are actually working with local copies!

Example to see the effect:
Code: Pascal  [Select][+][-]
  1. {$mode delphi}{$H+}
  2. function test1(t:string):integer;
  3. begin
  4.  Result := StringRefCount(t);
  5. end;
  6.  
  7. function test2(const t:string):integer;
  8. begin
  9.  Result := StringRefCount(t);
  10. end;
  11. var
  12.  s:string = 'test me';
  13. begin
  14.   writeln(test1(s+'1'));  // prints 2
  15.   writeln(test2(s+'2'));  // prints 1
  16. end.

The effect on the TEdit is of course the same as above.
Note here there is no leak, but it is easy to make it leak.
« Last Edit: October 15, 2019, 07:53:47 am by Thaddy »
Specialize a type, not a var.

PascalDragon

  • Hero Member
  • *****
  • Posts: 5446
  • Compiler Developer
Re: Is This A Leak?
« Reply #4 on: October 15, 2019, 09:07:24 am »
Both the Tedit and the string are managed types, indeed, but:
TEdit is not a managed type, at least not in the sense that a String is. It is bound to the lifetime of it's owner, but it's not actively managed by the compiler, so a const modifier doesn't change anything there.

Thaddy

  • Hero Member
  • *****
  • Posts: 14205
  • Probably until I exterminate Putin.
Re: Is This A Leak?
« Reply #5 on: October 15, 2019, 10:01:40 am »
Huh? It is refcounted?
Specialize a type, not a var.

Remy Lebeau

  • Hero Member
  • *****
  • Posts: 1312
    • Lebeau Software
Re: Is This A Leak?
« Reply #6 on: October 15, 2019, 07:11:46 pm »
Huh? It is refcounted?

Strings are reference-counted, yes.
Remy Lebeau
Lebeau Software - Owner, Developer
Internet Direct (Indy) - Admin, Developer (Support forum)

Thaddy

  • Hero Member
  • *****
  • Posts: 14205
  • Probably until I exterminate Putin.
Re: Is This A Leak?
« Reply #7 on: October 15, 2019, 08:04:20 pm »
« Last Edit: October 15, 2019, 08:06:05 pm by Thaddy »
Specialize a type, not a var.

del

  • Sr. Member
  • ****
  • Posts: 258
Re: Is This A Leak?
« Reply #8 on: October 15, 2019, 08:43:46 pm »
Without const/constref or var you are actually working with local copies!

I should probably be using const anyway unless there's a clear reason not to. I read somewhere that all objects passed as parameters in Free Pascal are passed as references. So you would want to protect them even if you weren't planning on changing them. And it's another way to leverage the compiler as a preemptive bug checker.

So ... if all objects are passed as references then the cases in which you would need the keyword "var" would be only those in which you altered the object itself, not just its member variables. Not sure how dynamic arrays fit into this scheme. If you pass one without using "var" and do a SetLength(), are you changing a local copy or are you changing the implicitly referenced original?

lucamar

  • Hero Member
  • *****
  • Posts: 4219
Re: Is This A Leak?
« Reply #9 on: October 15, 2019, 10:53:47 pm »
Not sure how dynamic arrays fit into this scheme. If you pass one without using "var" and do a SetLength(), are you changing a local copy or are you changing the implicitly referenced original?

You're changing a local copy. Test it for yourself:

Code: Pascal  [Select][+][-]
  1. {
  2.   TIntArray is declared as:
  3.     type TIntArray: array of Integer
  4. }
  5.  
  6. procedure TForm1.NoVar(AnArray: TIntArray);
  7. begin
  8.   SetLength(AnArray, 100);
  9. end;
  10.  
  11. procedure TForm1.WithVar(var AnArray: TIntArray);
  12. begin
  13.   SetLength(AnArray, 100);
  14. end;
  15.  
  16. procedure TForm1.Button1Click(Sender: TObject);
  17. { TheArray is a  private field: TheArray: TIntArray}
  18. begin
  19.   Memo1.Clear;
  20.   SetLength(TheArray, 5);
  21.   NoVar(TheArray);
  22.   Memo1.Lines.Add('After NoVar: %d', [Length(TheArray)]);
  23.   WithVar(TheArray);
  24.   Memo1.Lines.Add('After WithVar: %d', [Length(TheArray)]);
  25. end;

The result of that code is as expected:

Code: [Select]
After NoVar: 5
After WithVar: 100
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus/FPC 2.0.8/3.0.4 & 2.0.12/3.2.0 - 32/64 bits on:
(K|L|X)Ubuntu 12..18, Windows XP, 7, 10 and various DOSes.

del

  • Sr. Member
  • ****
  • Posts: 258
Re: Is This A Leak?
« Reply #10 on: October 16, 2019, 01:08:53 am »
I appreciate the effort you put into your answer. So the dynamic array is not treated like an instance of a class (and automatically passed by reference), or even as a pointer (the pointer is passed by value - but what it points to is the original data). OK - assuming that what I read (probably on the FP Wiki) is true - that objects / instances are automatically passed by reference, then I just need to go to the declaration code to see what kind of animal I'm dealing with. For speed purposes I suppose dynamic arrays should be passed as either plain var or const var. Here's what marcov said about that:

Quote
By reference parameters in Pascal don't base on pointer syntax like in C (& or *) .  You therefore don't need to micro manage it, but use proper by ref language constructs like const/var/out.

constref only makes a const argument forcedly by ref, but this is rarely used because strings and dynamic arrays already contain an implicit ref.

He's saying dynamic arrays (and strings!) contain an implicit ref. Which is a good thing - but your experiment showed "passing by value" behavior.

Anyhoo - good times. The thread I quoted marcov from:

https://forum.lazarus.freepascal.org/index.php?topic=46185.0

« Last Edit: October 16, 2019, 01:10:24 am by del »

Thaddy

  • Hero Member
  • *****
  • Posts: 14205
  • Probably until I exterminate Putin.
Re: Is This A Leak?
« Reply #11 on: October 16, 2019, 07:15:46 am »
@del
Both my example and Lucamar's example are not experiments:
They are common ways to show how the behavior differs between const/var/constref modifiers and no modifier at all.
QED if you want. If you read the documentation carefully (the real documentation, not a wiki!) both examples are exactly what should be expected.

Both of us, and others, have probably given similar examples in the past.
Specialize a type, not a var.

PascalDragon

  • Hero Member
  • *****
  • Posts: 5446
  • Compiler Developer
Re: Is This A Leak?
« Reply #12 on: October 16, 2019, 09:30:51 am »
Huh? It is refcounted?
Components are not refcounted by default. Yes, through TComponent they implement IInterface, but that is only valid if you access it through the VCLComObject member which is an interface. Class instances themselves are never reference counted by the compiler (only if you access them through an interface type the _AddRef and _Release methods are called at all), thus const vs. non-const does not matter in that regard.

lucamar

  • Hero Member
  • *****
  • Posts: 4219
Re: Is This A Leak?
« Reply #13 on: October 16, 2019, 11:39:06 am »
He's saying dynamic arrays (and strings!) contain an implicit ref. Which is a good thing - but your experiment showed "passing by value" behavior.

What Marco says is applicable mostly if your code does very low-level operations, say, if you write pure assembler or if you're used to the "C" way of doing things. Very, very basically (it's probably more complex) what the compiler does in the first case ("NoVar") is make a copy of the array or string and pass to the procedure a reference (pointer) to that copy, to avoid pushing the whole array/string into the stack (and other reasons, e.g. to be able to modify it inside the procedure without doing "gymnastics" with it).

The point here is that with var you're treating with a reference to the real variable but without it it's a reference to a "local" copy.
Turbo Pascal 3 CP/M - Amstrad PCW 8256 (512 KB !!!) :P
Lazarus/FPC 2.0.8/3.0.4 & 2.0.12/3.2.0 - 32/64 bits on:
(K|L|X)Ubuntu 12..18, Windows XP, 7, 10 and various DOSes.

del

  • Sr. Member
  • ****
  • Posts: 258
Re: Is This A Leak?
« Reply #14 on: October 16, 2019, 05:31:16 pm »
Very, very basically (it's probably more complex) what the compiler does in the first case ("NoVar") is make a copy of the array or string and pass to the procedure a reference (pointer) to that copy

And thus the riddle is explained. I didn't even think about that mechanism. Kinda cool - ownership of the temporary resource stays with the calling function.

 

TinyPortal © 2005-2018