fix: hang and wrong check states in data grid column selection on macOS

Fixes the reported hang (issue 2554): closing the column selection popup
with OK could run FormClose twice on macOS, freeing FCheckedColumns two
times. The resulting access violation raised the crash dialog inside a
paint cycle, freezing the application. FCheckedColumns is now freed in
FormDestroy, which runs exactly once.

While verifying that fix, a second bug in the same popup showed up:
checked columns were not applied correctly to the grid. The click
handler used ItemIndex to detect the toggled item, but on Cocoa the
check event fires before the list selection is updated, so the wrong
item was added or removed. The handler now syncs the check states of
all displayed items instead.

Author: twinn1013

source/column_selection.pas

@@ -150,20 +150,20 @@ procedure TfrmColumnSelection.editFilterButtonClick(Sender: TObject);
 }
 procedure TfrmColumnSelection.chklistColumnsClickCheck(Sender: TObject);
 var
-  i : Integer;
+  i, CheckedIndex : Integer;
   AllSelected, NoneSelected : Boolean;
-  FocusedItem: String;
-  FocusedItemIndex: Integer;
 begin
-  // Add or remove clicked item from list
-  if chklistColumns.ItemIndex > -1 then begin
-    FocusedItem := chklistColumns.Items[chklistColumns.ItemIndex];
-    if chklistColumns.Checked[chklistColumns.ItemIndex] then begin
-      FCheckedColumns.Add(FocusedItem)
+  // Sync check states of all displayed items into FCheckedColumns. Using
+  // ItemIndex to detect the clicked item would be wrong on macOS, where the
+  // check event fires before the list selection is updated. See issue 2554.
+  for i:=0 to chklistColumns.Items.Count-1 do begin
+    CheckedIndex := FCheckedColumns.IndexOf(chklistColumns.Items[i]);
+    if chklistColumns.Checked[i] then begin
+      if CheckedIndex = -1 then
+        FCheckedColumns.Add(chklistColumns.Items[i]);
     end else begin
-      FocusedItemIndex := FCheckedColumns.IndexOf(FocusedItem);
-      if FocusedItemIndex > -1 then
-        FCheckedColumns.Delete(FocusedItemIndex);
+      if CheckedIndex > -1 then
+        FCheckedColumns.Delete(CheckedIndex);
     end;
   end;
 
@@ -225,6 +225,7 @@ procedure TfrmColumnSelection.FormDestroy(Sender: TObject);
 begin
   AppSettings.WriteInt(asColumnSelectorWidth, ScaleFormToDesign(Width));
   AppSettings.WriteInt(asColumnSelectorHeight, ScaleFormToDesign(Height));
+  FCheckedColumns.Free;
 end;
 
 
@@ -243,8 +244,9 @@ procedure TfrmColumnSelection.FormDeactivate(Sender: TObject);
 procedure TfrmColumnSelection.FormClose(Sender: TObject; var Action:
     TCloseAction);
 begin
+  // FormClose can run twice when the form is closed by OK and afterwards
+  // deactivated - free FCheckedColumns in FormDestroy only. See issue 2554.
   Action := caFree;
-  FCheckedColumns.Free;
 end;