LCL: fix crash when using freeing a component of a TRadioGroup or TCheckGroup. Issue #40261.

lcl/extctrls.pp

@@ -730,6 +730,9 @@ type
     procedure SetItemIndex(Value: integer);
     function GetItemIndex: integer;
     procedure CheckItemIndexChanged; virtual;
+
+    procedure Notification(AComponent: TComponent; Operation: TOperation); override;
+
   public
     constructor Create(TheOwner: TComponent); override;
     destructor Destroy; override;
@@ -853,6 +856,9 @@ type
     procedure WriteData(Stream: TStream);
     procedure Loaded; override;
     procedure DoOnResize; override;
+
+    procedure Notification(AComponent: TComponent; Operation: TOperation); override;
+
   public
     constructor Create(TheOwner: TComponent); override;
     destructor Destroy; override;

lcl/include/customcheckgroup.inc

@@ -53,13 +53,22 @@ var
   b: TByteDynArray;
   i: Integer;
 begin
-  SaveCheckStates(b);
+  if (FCheckgroup.FButtonList.Count < FCheckgroup.Items.Count) then
-
+    //CheckBox has already been removed from FButtonList (via Components[x].Free)
-  inherited Delete(AIndex);
+    //All necessesary info for the checkboxes are already stored in FButtonList
-
+    //and FButtonList won't be altered in FCheckGroup.UpdateItems,
-  for i:= AIndex to High(b)-1 do b[i] := b[i+1];
+    //so no need for SaveCheckStates/RestoreCheckStates.
-  SetLength(b, Length(b)-1);
+    //(Also in this scenario Items and FButtonList are out of sysnc, so SaveCheckStates
-  RestoreCheckStates(b);
+    //will cause an EListError.)
+    //Issue #40261
+    inherited Delete(AIndex)
+  else begin
+    SaveCheckStates(b);
+    inherited Delete(AIndex);
+    for i:= AIndex to High(b)-1 do b[i] := b[i+1];
+    SetLength(b, Length(b)-1);
+    RestoreCheckStates(b);
+  end;
 end;
 
 procedure TCheckGroupStringList.InsertItem(Index: Integer; const S: string; O: TObject);
@@ -95,6 +104,7 @@ procedure TCheckGroupStringList.SaveCheckStates(out AStates: TByteDynArray);
 var
   i: Integer;
 begin
+  Assert(FCheckgroup.FButtonList.Count = FCheckgroup.Items.Count); //see TCheckGroupStringList.Delete()
   SetLength(AStates, FCheckgroup.Items.Count);
   for i:=0 to FCheckgroup.Items.Count-1 do begin
     AStates[i] := 0;
@@ -218,7 +228,8 @@ begin
     while (FButtonList.Count<FItems.Count) do begin
       CheckBox := TCheckBox.Create(Self);
       with CheckBox do begin
-        Name:='CheckBox'+IntToStr(FButtonList.Count);
+        //Don't set name here, it may already exist if Components[x].Free was used
+        //Issue #40261
         AutoSize := False;
         BorderSpacing.CellAlignHorizontal:=ccaLeftTop;
         BorderSpacing.CellAlignVertical:=ccaCenter;
@@ -233,10 +244,15 @@ begin
       end;
       FButtonList.Add(CheckBox);
     end;
+
+
     for i:=0 to FItems.Count-1 do begin
       CheckBox:=TCheckBox(FButtonList[i]);
       CheckBox.Caption:=FItems[i];
+      CheckBox.Name := '';
     end;
+    for i:=0 to FButtonList.Count-1 do
+      TCheckBox(FButtonList[i]).Name:='CheckBox'+IntToStr(i);
   finally
     FUpdatingItems:=false;
   end;
@@ -395,6 +411,27 @@ begin
   inherited DoOnResize;
 end;
 
+procedure TCustomCheckGroup.Notification(AComponent: TComponent;
+  Operation: TOperation);
+var
+  Idx: Integer;
+begin
+  inherited Notification(AComponent, Operation);
+  if (Operation = opRemove) and (Assigned(FButtonList)) then
+  begin
+    Idx := FButtonList.IndexOf(AComponent);
+    //if triggered by Items.Delete, then
+    // * it will always be the last CheckBox('s) that will be removed
+    // * Items.Count will already have been decremented, so Idx will be equal to Items.Count
+    if (Idx <> -1) and (Idx < Items.Count) then
+    begin
+      FButtonList.Delete(Idx);
+      AComponent.Name := '';  //otherwise we get duplicate name error in UpdateItems
+      Items.Delete(Idx);
+    end;
+  end;
+end;
+
 function TCustomCheckGroup.Rows: integer;
 begin
   if FItems.Count>0 then

lcl/include/radiogroup.inc

@@ -180,7 +180,7 @@ begin
       ARadioButton := TRadioButton.Create(Self);
       with ARadioButton do
       begin
-        Name := 'RadioButton'+IntToStr(FButtonList.Count);
+        //Don't set Name here, it may already exist if Components[x].Free was used. Issue #40261
         OnClick := @Self.Clicked;
         OnChange := @Self.Changed;
         OnEnter := @Self.ItemEnter;
@@ -228,7 +228,11 @@ begin
         ARadioButton := TRadioButton(FButtonList[i]);
         ARadioButton.Checked := (i = FItemIndex);
         ARadioButton.Visible := true;
+        ARadioButton.Name := '';
       end;
+      for i:=0 to FButtonList.Count-1 do
+        TRadioButton(FButtonList[i]).Name:='RadioButton'+IntToStr(i);
+
       //FHiddenButton must remain the last item in Controls[], so that Controls[] is in sync with Items[]
       Self.RemoveControl(FHiddenButton);
       Self.InsertControl(FHiddenButton);
@@ -460,6 +464,26 @@ begin
   if Assigned (FOnSelectionChanged) then FOnSelectionChanged(Self);
 end;
 
+procedure TCustomRadioGroup.Notification(AComponent: TComponent;
+  Operation: TOperation);
+var
+  Idx: Integer;
+begin
+  inherited Notification(AComponent, Operation);
+  if (Operation = opRemove) and (Assigned(FButtonList)) then
+  begin
+    Idx := FButtonList.IndexOf(AComponent);
+    //if triggered by Items.Delete, then
+    // * it will always be the last radiobutton(s) that will be removed
+    // * Items.Count will already have been decremented, so Idx will be equal to Items.Count
+    if (Idx <> -1) and (Idx < Items.Count) then
+    begin
+      FButtonList.Delete(Idx);
+      AComponent.Name := '';
+      Items.Delete(Idx);
+    end;
+  end;
+end;
 {------------------------------------------------------------------------------
   Method: TCustomRadioGroup.CanModify
   Params:  none