[PATCH] TCheckBox.Onclick might cause infinite recursion, because of Delphi semantics used for event handlers.

UPDATE: LCL patch and test programs provided in the comment below: #39870 (comment 1093373752)

  • Lazarus/FPC Version: 2.2.3 / 3.2.3

36170-bis_Sources.zip

This code, after clicking both checkboxes, leads to infinite recursion and stack overflow. According to #36170 (closed) this IS a bug and should be fixed.

The technical cause of such a behavior is simplistic guard-less events programming AND attempt at Delphi compatiblity in LCL.

  TForm1 = class(TForm)
    CheckBox1: TCheckBox;
    CheckBox2: TCheckBox;
    procedure CheckBox1Click(Sender: TObject);
    procedure CheckBox2Click(Sender: TObject);
  end;

procedure TForm1.CheckBox1Click(Sender: TObject);
begin
  CheckBox1.Checked := True;
  CheckBox2.Checked := False;
end;

procedure TForm1.CheckBox2Click(Sender: TObject);
begin
  CheckBox2.Checked := True;
  CheckBox1.Checked := False;
end;

Currenty this code would blow the program after both checkboxes clicked.

According to #36170 (closed) (2 years ago) - this simple event handlers are okay and should just work.

According to discusison in #33076 (closed) (4 years ago) - the Delphi semantics has to be preserved and OnClick event fired when tick mark state changes.

You can not have both! You have to make a choice, and then clearly document it in both sources and documentations, for both users (application programmers) and toolkit programmers. Otherwise those bugs would remain unsolved.

Option 1: settle on VCL compatibility. Make OnClick fired for CheckBox1.State := ... and Checkbox1.Checked := not Checkbox1.Checked code (which would make it rather similar to OnChange event). Make it responsibility of users to insert guarding boilerplate in their event handlers, that can trigger reentrancy. Create documentation page for TCustomCheckbox.OnClick and document the deviation from TControl.OnClick template.

Option 2: settle on simplistic events handlers code. Make CheckBox1.State := ... and Checkbox1.Checked := ... code NEVER fire OnCLick events. Create documentation page for TCustomCheckbox.OnClick and explicitly document it. Make Delphi projects converter to change OnClick events to OnChange events automatically.

Option 3: introduce runtime switch like class var TCustomCheckBox.VCL_OnClick_Emulation: boolean and let users opt in or opt out like they see fit for their projects. This would also provide a much easier change for Delphi projects converter. THe drawback would be some extra burden upon LCL mantainers. Create documentation page for TCustomCheckbox.OnClick and document both behaviors and how to switch them.

Code changes for all those options are technically trivial, but the decision which way to go (policy) must be made.

P.S. This is not directly related to #39869 (closed), which is focused on TAction compatibility, however resolving TAction problem in 39869 had inadvertently re-implemented #33076 (closed) and re-triggered #36170 (closed). Now we have an infinite recursion between two bugreports.

Edited by Arioch The