Edrean Ernst Edrean Ernst - 1 month ago 7
Android Question

Freeing buttons in a list in OnClick

What I'm trying to achieve, in simplified form, is to create a list of dynamically created buttons. When clicking on one of the buttons it should be removed from the list and its object should be freed. My approach is :


  • Create a
    TList<TButton>

  • Create a couple of
    TButton
    objects and add them to the
    TList<TButton>

  • Assign the
    Form
    as the
    Parent
    for each of the created
    TButton
    objects

  • Assign a
    Position
    for each of the created
    TButton
    objects

  • Assign an
    OnClick
    handler method to each of the created
    TButton
    objects

  • The
    OnClick
    handler sets the
    Sender
    TButton
    's
    Parent
    to
    nil
    and deletes it from the
    TList<TButton>
    , so that ARC can free the
    TButton
    object that was clicked on.



When I click on one of the dynamically created buttons, I get a "Segmentation Fault". I suspect it is because I am freeing the
TButton
object in its own
OnClick
handler and the class is trying to do some other stuff with it after my handler.

I have tested this on Android. I assume the same will happen on iOS, or any other ARC platform for that matter.

Is there a better/right way to do this, or another approach I should be following to get it working the way I want?

Here is some example code. It is for a form with one design-time button (
Button1
) on it. Clicking this button repeatedly dynamically creates new buttons and adds them to the list.

unit Unit2;

interface

uses
System.SysUtils, System.Types, System.UITypes, System.Classes, System.Variants,
FMX.Types, FMX.Controls, FMX.Forms, FMX.Graphics, FMX.Dialogs,
FMX.Controls.Presentation, FMX.StdCtrls, System.Generics.Collections;

type
TForm2 = class(TForm)
Button1: TButton;
procedure Button1Click(Sender: TObject);
procedure FormCreate(Sender: TObject);
private
ButtonList : TList<TButton>;
procedure ButtonClick(Sender: TObject);
{ Private declarations }
public
{ Public declarations }
end;

var
Form2: TForm2;

implementation

{$R *.fmx}

procedure TForm2.ButtonClick(Sender: TObject);
var
pos : Integer;
begin
pos := ButtonList.IndexOf(TButton(Sender));
TButton(Sender).Parent := nil;
ButtonList.Delete(pos);
end;

procedure TForm2.FormCreate(Sender: TObject);
begin
ButtonList := TList<TButton>.Create;
end;

procedure TForm2.Button1Click(Sender: TObject);
var
pos : Integer;
begin
pos := ButtonList.Add(TButton.Create(nil));
ButtonList.Items[pos].Parent := Form2;
ButtonList.Items[pos].Position.Y := 50 * ButtonList.Count;
ButtonList.Items[pos].OnClick := ButtonClick;
end;

end.

Answer

When I click on one of the dynamically created buttons, I get a "Segmentation Fault". I suspect it is because I am freeing the TButton object in its own OnClick handler and the class is trying to do some other stuff with it after my handler.

That is exactly what is happening. After the event handler exits, the RTL still needs access to the button object to finish processing the click and message handling. It is never safe to destroy a UI object from inside of its own events. So you have to make sure the object remains alive during event handling.

I have tested this on Android. I assume the same will happen on iOS, or any other ARC platform for that matter.

Yes. And it would also happen on non-ARC platforms if you tried to Free the button explicitly, eg:

procedure TForm2.ButtonClick(Sender: TObject);
var
  btn: TButton;
begin
  btn := TButton(Sender);
  ButtonList.Remove(btn);
  {$IFDEF AUTOREFCOUNT}
  btn.Parent := nil;
  {$ELSE}
  btn.Free;
  {$ENDIF}
end;

Is there a better/right way to do this, or another approach I should be following to get it working the way I want?

You could have the OnClick handler post an asynchronous message to the main thread (such as by calling TThread.Queue() inside of TThread.CreateAnonymousThread() or TTask.Run()) and then exit immediately, letting the message handler free the button at a later time when the button is no longer being used, eg:

procedure TForm2.ButtonClick(Sender: TObject);
var
  btn: TButton
begin
  btn := TButton(Sender);
  ButtonList.Remove(btn);
  TThread.CreateAnonymousThread(
    procedure
    begin
      TThread.Queue(nil,
        procedure
        begin
          btn.DisposeOf;
        end
      );
    end
  ).Start;
end;

Or, you could move the button object to another list and then start a short timer to run through that list freeing its objects, eg:

unit Unit2;

interface

uses
  System.SysUtils, System.Types, System.UITypes, System.Classes, System.Variants,
  FMX.Types, FMX.Controls, FMX.Forms, FMX.Graphics, FMX.Dialogs,
  FMX.Controls.Presentation, FMX.StdCtrls, System.Generics.Collections;

type
  TForm2 = class(TForm)
    Button1: TButton;
    Timer1: TTimer;
    procedure Button1Click(Sender: TObject);
    procedure FormCreate(Sender: TObject);
    procedure Timer1Timer(Sender: TObject);
  private
    ButtonList : TList<TButton>;
    DisposeList : TList<TButton>;
    procedure ButtonClick(Sender: TObject);
    { Private declarations }
  public
    { Public declarations }
  end;

var
  Form2: TForm2;

implementation

{$R *.fmx}

procedure TForm2.ButtonClick(Sender: TObject);
var
  btn: TButton;
begin
  btn := TButton(Sender);
  ButtonList.Remove(btn);
  DisposeList.Add(btn);
  Timer1.Enabled := true;
end;

procedure TForm2.FormCreate(Sender: TObject);
begin
  ButtonList := TList<TButton>.Create;
  DisposeList := TList<TButton>.Create;
end;

procedure TForm2.Button1Click(Sender: TObject);
var
  btn: TButton;
begin
  btn := TButton.Create(nil);
  ButtonList.Add(btn);
  btn.Parent := Self;
  btn.Position.Y := 50 * ButtonList.Count;
  btn.OnClick := ButtonClick;
end;

procedure TForm2.Timer1Timer(Sender: TObject);
var
  btn: TButton;
begin
  Timer1.Enabled := False;
  for btn in DisposeList do
    btn.DisposeOf;
  DisposeList.Clear;
end;

end.