Search code examples
delphifiremonkeydelphi-10.4-sydney

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


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.

Solution

  • 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.