Why does freeing a member component in a class destructor cause an EInvalidPointer error when the application is closed?

Question

Here is a class I created to add a TLabel to a TTrackBar. The label shows the value of trackbar when dragged and then fades out. An instance is created at runtime and the parent set to the form. It works fine but gives an error when the application is closed if the trackbar still exists. However, there's no problem if the trackbar is freed at runtime and then the application closed. When debugging that line when the application closes (FLabel.Free;) I see that FLabel and the data in it still exists, but it still gives that error. I'm concerned that if I simply remove that line then there'll be a memory leak when freeing the object at run time. I've tried changing it to if Assigned(FLabel) then FLabel.Free; but with no change. I know it must have something to do with the fact that the label's parent has been set.

unit TrackBarLabelUnit;

interface

uses
  System.Types, System.Classes, System.SysUtils, FMX.Types, FMX.StdCtrls,
  FMX.Controls;

type
  TValueToString = function(AValue : Single) : String of object;

  TTrackBarLabel = class(TTrackBar)
  private
    FLabel : TLabel;
    FSuffix : String;
    FTimer : TTimer;
    FOffset : Integer;
    FValueToString : TValueToString;

    procedure TimerTimer(Sender: TObject);
  protected
    procedure ParentChanged; override;
    procedure DoTracking; override;
  public
    constructor Create(AOwner: TComponent); override;
    destructor Destroy; override;

    property Suffix : String read FSuffix write FSuffix;
    property LabelOffset : Integer read FOffset write FOffset;
    property ValueToString : TValueToString write FValueToString;
  end;

implementation

constructor TTrackBarLabel.Create(AOwner: TComponent);
begin
  inherited;
  FLabel := TLabel.Create(nil);
  FLabel.Visible := False;
  FTimer := TTimer.Create(nil);
  FTimer.Interval := 100;
  FTimer.Enabled := False;
  FTimer.OnTimer := TimerTimer;
  FSuffix := '';
  FOffset := 22;
end;

destructor TTrackBarLabel.Destroy;
begin
  FLabel.Free; // EInvalidPointer error here when application is closed
  FTimer.Free;
  inherited Destroy;
end;

procedure TTrackBarLabel.ParentChanged;
begin
  inherited;
  FLabel.Parent := Parent;
end;

procedure TTrackBarLabel.DoTracking;
begin
  inherited;

  if not Assigned(Thumb) then Exit;

  FLabel.Visible := True;
  FLabel.Tag := 10;
  FLabel.Opacity := 1;

  if Assigned(FValueToString) then
    FLabel.Text := FValueToString(Value) + FSuffix
  else
    FLabel.Text := FloatToStrF(Value, ffFixed, 12, 1) + FSuffix;

  if Orientation = TOrientation.Horizontal then begin
    FLabel.Position.X := Position.X + Thumb.Position.X +
                         (Thumb.Width - FLabel.Width) * 0.5;
    FLabel.Position.Y := Position.Y + FOffset;
    FLabel.TextSettings.HorzAlign := TTextAlign.Center;
  end else begin
    FLabel.Position.X := Position.X + FOffset;
    FLabel.Position.Y := Position.Y + Thumb.Position.Y - 2;
    FLabel.TextSettings.HorzAlign := TTextAlign.Leading;
  end;

  FTimer.Enabled := False;
  FTimer.Enabled := True;
end;

procedure TTrackBarLabel.TimerTimer(Sender: TObject);
begin
  FLabel.Tag := FLabel.Tag - 1;
  FLabel.Opacity := FLabel.Tag * 0.2;
  if FLabel.Tag < 0 then begin
    FLabel.Visible := False;
    FTimer.Enabled := False;
  end;
end;

end.

Andreas Rejbrand · Accepted Answer · 2020-12-22T21:03:24.957

7

Most often, an invalid-pointer exception means that you try to free an object twice.

The problem in this case is that a control frees its children when it is freed. So when the form is freed, it also frees the TLabel. Thus, when your TTrackBarLabel.Destroy is executed, your FLabel is a dangling pointer, and you mustn't do FLabel.Free.

All Delphi developers know that a component frees its owned components when it is freed. It is a less known fact that a control also frees its children.

In your case, you can simply remove the FLabel.Free. However, this will cause a memory leak if you never set the FLabel's Parent property.

To make sure the label is automatically freed when the track bar is, make the track bar the owner of the label:

  FLabel := TLabel.Create(Self);

As an aside, your suggestion

if Assigned(FLabel) then
  FLabel.Free;

won't help, because FLabel is a dangling pointer when the error occurs (it's not nil).

Also, in Delphi, you never write

if Assigned(FLabel) then
  FLabel.Free;

because TObject.Free basically does if Assigned then Destroy, so

if Assigned(FLabel) then
  FLabel.Free;

means

if Assigned(FLabel) then
  if Assigned(FLabel) then
    FLabel.Destroy;

which is very silly.

edited Dec 22 '20 at 21:03

answered Dec 22 '20 at 13:02

Andreas Rejbrand

105,602
8
282
384

Thanks. So in summary I shouldn't need to free it myself and to make sure of that I make the trackbar the owner of the label. I did do that originally, but wanted to free it myself to make sure it was freed. I read somewhere that creating the label with nil would mean that I would be responsible to free it myself, but that doesn't seem the case since the form frees its children anyway. Why isn't there an issue with the TTimer? I know that the parent isn't set like the label, but if I create it with the trackbar as owner I still don't get any issues when it's freed. – XylemFlow Dec 23 '20 at 09:21
1

@XylemFlow: (1) Correct. Remove your `FLabel.Free` and write `TLabel.Create(Self)` to make the track bar the owner of the label. (2) Indeed, "everyone" knows that a component frees its owned (`Owner`) components when it is freed, but it is not as well known that a control frees its children (`Parent`) when it is freed. (3) The `TTimer` is a component (`TComponent`), but not a control (`TControl`). It isn't something displayed on screen; it has no parent. If you create it with the track bar as its owner it will indeed be freed when the track bar is, but when you do `FTimer.Free` ... – Andreas Rejbrand Dec 23 '20 at 10:45
... the owner (the track bar) is notified about this and the timer is removed from the owner's list of owned components. – Andreas Rejbrand Dec 23 '20 at 10:46
Thanks. It sounds like I don't need the destructor at all then. I guess that a destructor is only really needed to free objects that don't have an owner or to clear an array. – XylemFlow Dec 23 '20 at 14:08
Technically, you need the destructor, but you don't need to call it yourself, because the run-time library will do that for you (in this case, when the owner is freed). – Andreas Rejbrand Dec 23 '20 at 14:09
To avoid the dangling pointer issue, call `FLabel.FreeNotification(Self)` and then override the virtual `Notification()` to set `FLabel := nil` when the label is destroyed. If `Self` is used as the Owner, `FreeNotification()` is called automatically for you, but you would still need to implement `Notification()` – Remy Lebeau Dec 25 '20 at 18:19
@RemyLebeau: True, but in most situations, the simpler approach outlined in my A is of course quite enough. But it is a good comment for those having special needs. – Andreas Rejbrand Dec 25 '20 at 18:24
@AndreasRejbrand in a way, this is a special case, since the Label has a different `Parent` than its `Owner`. Like you said, the `Parent` could free the Label before freeing the TrackBar, so best not to leave a dangling pointer to chance. – Remy Lebeau Dec 25 '20 at 18:41
@RemyLebeau: Yes, you might refer to `FLabel` in other places than the destructor. That might be dangerous, and if so you might really do need the notification. – Andreas Rejbrand Dec 25 '20 at 19:18

Why does freeing a member component in a class destructor cause an EInvalidPointer error when the application is closed?

1 Answers1