Recent

Author Topic: Lazarus trank: stack overflow for new revisions  (Read 1541 times)

zoltanleo

  • Sr. Member
  • ****
  • Posts: 486
Lazarus trank: stack overflow for new revisions
« on: September 25, 2022, 07:32:24 pm »
Hi folks

I am getting an overflow error in my old project after updating lazarus to newer versions.
Code: Pascal  [Select][+][-]
  1. Project project1 raised exception class 'External: STACK OVERFLOW'.
  2.  At address 10000E3BF

The project compiles and builds successfully. In the test project, this happens inside the loop (this happens in the onclick event for each button):
Code: Pascal  [Select][+][-]
  1.   for i:= 0 to Pred(Panel1.ControlCount) do
  2.     if TControl(Panel1.Controls[i]).InheritsFrom(TToggleBox) then
  3.       TToggleBox(Panel1.Controls[i]).State:= cbUnchecked;

The error is observed for Lazarus revision: main-2_3-2511-gfee5b58ce2 and older.

The error is reproduced for win34/64 and gtk2_x64 (I didn't check for qt and darwin)

What happened?

Updated:
If I add an additional flag that prevents recursion in changing the component, then no error occurs

Code: Pascal  [Select][+][-]
  1.   private
  2.     FInChanging: Boolean;
  3.  
  4. procedure TForm1.TglBoxClick(Sender: TObject);
  5. var
  6.   i: PtrInt = 0;
  7. begin
  8.   if FInChanging then Exit;
  9.   FInChanging := True;
  10.  
  11.   if not TObject(Sender).InheritsFrom(TToggleBox) then Exit;
  12.  
  13.   try
  14.     Memo1.Lines.Add('');
  15.     Memo1.Lines.Add(Format('====== %s.Click ======',[TToggleBox(Sender).Name]));
  16.  
  17.     Memo1.Lines.Add('');
  18.  
  19.  
  20.     for i:= 0 to Pred(Panel1.ControlCount) do
  21.       if TControl(Panel1.Controls[i]).InheritsFrom(TToggleBox) then
  22.         begin
  23.           TToggleBox(Panel1.Controls[i]).State:= cbUnchecked;
  24.           Memo1.Lines.Add(Format('Panel1.Controls[%d] State property changed to ''cbUnchecked'' value (%s)',
  25.           [i, FormatDateTime('hh:nn:ss.zzz',Now)]));
  26.         end;
  27.  
  28.     TToggleBox(Sender).State:= cbChecked;
  29.     Memo1.Lines.Add(Format('%s State property changed to ''cbChecked'' value (%s)',
  30.     [TToggleBox(Sender).Name, FormatDateTime('hh:nn:ss.zzz',Now)]));
  31.   finally
  32.     FInChanging:= False;
  33.   end;
  34. end;
                                     
« Last Edit: September 25, 2022, 08:19:15 pm by zoltanleo »
Win10 LTSC x64/Deb 11 amd64(gtk2/qt5)/Darwin Cocoa (Monterey):
Lazarus x32/x64 2.3(trunk); FPC 3.3.1 (trunk), FireBird 3.0.10; IBX by TonyW

Sorry for my bad English, I'm using translator ;)

paweld

  • Hero Member
  • *****
  • Posts: 970
Re: Lazarus trank: stack overflow for new revisions
« Reply #1 on: September 25, 2022, 08:29:13 pm »
Modifying the state of TToggleBox triggers the OnClick event, so set TToggleBox.OnClick to nil when changing the state, or try to avoid changing the status in the OnClick event, as it falls into an infinite loop.
Code: Pascal  [Select][+][-]
  1. procedure TForm1.TglBoxClick(Sender: TObject);
  2. var
  3.   i: PtrInt = 0;
  4. begin
  5.   if not TObject(Sender).InheritsFrom(TToggleBox) then Exit;
  6.  
  7.   Memo1.Lines.Add('');
  8.   Memo1.Lines.Add(Format('====== %s.Click ======',[TToggleBox(Sender).Name]));
  9.  
  10.   Memo1.Lines.Add('');
  11.  
  12.  
  13.   for i:= 0 to Pred(Panel1.ControlCount) do
  14.     if TControl(Panel1.Controls[i]).InheritsFrom(TToggleBox) then
  15.       begin
  16.         TToggleBox(Panel1.Controls[i]).OnClick:= nil;   //<--Add this
  17.         TToggleBox(Panel1.Controls[i]).State:= cbUnchecked;
  18.         TToggleBox(Panel1.Controls[i]).OnClick:= @TglBoxClick;   //<--and this
  19.         Memo1.Lines.Add(Format('Panel1.Controls[%d] State property changed to ''cbUnchecked'' value (%s)',
  20.         [i, FormatDateTime('hh:nn:ss.zzz',Now)]));
  21.       end;
  22.  
  23.   TToggleBox(Sender).OnClick:= nil;   //<--and this
  24.   TToggleBox(Sender).State:= cbChecked;
  25.   TToggleBox(Sender).OnClick:= @TglBoxClick;   //<--and this
  26.   Memo1.Lines.Add(Format('%s State property changed to ''cbChecked'' value (%s)',
  27.   [TToggleBox(Sender).Name, FormatDateTime('hh:nn:ss.zzz',Now)]));
  28. end;            
  29.  
Best regards / Pozdrawiam
paweld

PascalDragon

  • Hero Member
  • *****
  • Posts: 5446
  • Compiler Developer
Re: Lazarus trank: stack overflow for new revisions
« Reply #2 on: September 25, 2022, 09:04:34 pm »
Or you could check inside the loop whether the control is the Sender and not change its State then.


zoltanleo

  • Sr. Member
  • ****
  • Posts: 486
Re: Lazarus trank: stack overflow for new revisions
« Reply #4 on: September 25, 2022, 10:14:19 pm »
Thanks for answers.

Can I consider the current behavior of the component as normal?
Win10 LTSC x64/Deb 11 amd64(gtk2/qt5)/Darwin Cocoa (Monterey):
Lazarus x32/x64 2.3(trunk); FPC 3.3.1 (trunk), FireBird 3.0.10; IBX by TonyW

Sorry for my bad English, I'm using translator ;)

Arioch

  • Sr. Member
  • ****
  • Posts: 421
Re: Lazarus trank: stack overflow for new revisions
« Reply #5 on: September 25, 2022, 10:14:28 pm »
Or you could check inside the loop whether the control is the Sender and not change its State then.

or use OnChange ratehr than OnClick and turn Delphi emulation off

Bart

  • Hero Member
  • *****
  • Posts: 5275
    • Bart en Mariska's Webstek
Re: Lazarus trank: stack overflow for new revisions
« Reply #6 on: September 25, 2022, 11:20:00 pm »
Is this documented on the 2.4 release notes page?

Arioch

  • Sr. Member
  • ****
  • Posts: 421
Re: Lazarus trank: stack overflow for new revisions
« Reply #7 on: September 25, 2022, 11:46:16 pm »
And as this topic was brought, it would've been nice if ppl test a patch for TDBCheckBox on different OSes/toolkits

https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39917

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: Lazarus trank: stack overflow for new revisions
« Reply #8 on: September 26, 2022, 12:43:26 am »
I made the suggestion and also tested it locally to include a MODIFIED property in this control.

A few other controls already have this PROPERTY for the same exact reason.

When a software set is done the events can test the MODIFIED property which should be set to FALSE indicated that the user did not trigger it. Otherwise, it's set to true.

 This property is READ/WRITE so when it loses focus it will retain its value so that later on you can scan some controls on the form for changes and update some user file or whatever.

 Most of the time what I do is use the OnEnter event and clear this property first if needed or simply reload it software wise which will clear it.

 To each their own I guess, but we really should follow some order and I think the Modified property is appropriate for this.


The only true wisdom is knowing you know nothing

zoltanleo

  • Sr. Member
  • ****
  • Posts: 486
Re: Lazarus trank: stack overflow for new revisions
« Reply #9 on: September 26, 2022, 09:35:42 am »
Modifying the state of TToggleBox triggers the OnClick event, so set TToggleBox.OnClick to nil when changing the state, or try to avoid changing the status in the OnClick event, as it falls into an infinite loop.
Thank you. This is a good idea. I did it this way:

Code: Pascal  [Select][+][-]
  1.     for i:= 0 to Pred(Panel1.ControlCount) do
  2.       if TControl(Panel1.Controls[i]).InheritsFrom(TToggleBox) then
  3.         begin
  4.           TToggleBox(Panel1.Controls[i]).OnClick:= nil;
  5.           if TToggleBox(Panel1.Controls[i]).Equals(TToggleBox(Sender))
  6.             then
  7.               TToggleBox(Panel1.Controls[i]).State:= cbChecked
  8.             else
  9.               TToggleBox(Panel1.Controls[i]).State:= cbUnchecked;
  10.  
  11.           TToggleBox(Panel1.Controls[i]).OnClick:= @TglBoxClick;
  12.  
  13.         end;
Win10 LTSC x64/Deb 11 amd64(gtk2/qt5)/Darwin Cocoa (Monterey):
Lazarus x32/x64 2.3(trunk); FPC 3.3.1 (trunk), FireBird 3.0.10; IBX by TonyW

Sorry for my bad English, I'm using translator ;)

zoltanleo

  • Sr. Member
  • ****
  • Posts: 486
Re: Lazarus trank: stack overflow for new revisions
« Reply #10 on: September 26, 2022, 09:38:55 am »
I thank everyone who participated in the discussion. I hope this behavior of the component will not change in future versions of Lazarus. Otherwise, I will again have to redo the code that worked well before  ;)
Win10 LTSC x64/Deb 11 amd64(gtk2/qt5)/Darwin Cocoa (Monterey):
Lazarus x32/x64 2.3(trunk); FPC 3.3.1 (trunk), FireBird 3.0.10; IBX by TonyW

Sorry for my bad English, I'm using translator ;)

Arioch

  • Sr. Member
  • ****
  • Posts: 421
Re: Lazarus trank: stack overflow for new revisions
« Reply #11 on: September 26, 2022, 01:44:10 pm »
Just make your mind, if you want to react on change or on user click, then turn Delphi emulation off and use the event you need.

Delphi VCL has only OnChange (albeit wrongly named).

Arioch

  • Sr. Member
  • ****
  • Posts: 421
Re: Lazarus trank: stack overflow for new revisions
« Reply #12 on: September 26, 2022, 01:56:26 pm »
I made the suggestion and also tested it locally to include a MODIFIED property in this control.

A few other controls already have this PROPERTY for the same exact reason.

did you put your patch to https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues ?

well, "modified" name is rather ambiguous. There are a lot of reasons why a control could be modified. Databse compontent, TAction components... Direct changes in the platform/OS outside of LCL framework.

also, imagine a group of radiobuttons, a user clicks rb1 and rb2 looses its tick. Is rb2 modified by user or by rb1? what should be a concept here?

if to imagine some conceptually consistent design - then perhaps all such changes should had included "originator control" parameter.

....which could've been done in original message-based OOP like in Windows GDI or Smalltak, or with member procedure calls, but can hardly be made with properties assignment like used everywhere in LCL//VCL

Code: Pascal  [Select][+][-]
  1. RadioButton2.Checked := True<nil>; // changed by user
  2. RadioButton2.Checked := False<RadioButton1>; // changed by another RB
  3.  
hello, tuples :-D

Maybe VCL/LCL could've been designed in a way it never uses properties directly, leaving them as solely external API, and for internal transaction always calling setter procedures with additional parameters. But it is easy today to talk how things should've been done in 1995 :-)

ASerge

  • Hero Member
  • *****
  • Posts: 2223
Re: Lazarus trank: stack overflow for new revisions
« Reply #13 on: September 26, 2022, 08:07:13 pm »
Code: Pascal  [Select][+][-]
  1.     for i:= 0 to Pred(Panel1.ControlCount) do
  2.       if TControl(Panel1.Controls[i]).InheritsFrom(TToggleBox) then
  3.       ...
  4.  
I suggest using modern language features:
Code: Pascal  [Select][+][-]
  1. procedure TForm1.TglBoxClick(Sender: TObject);
  2. const
  3.   CStateWhenSender: array [Boolean] of TCheckBoxState = (cbUnchecked, cbChecked);
  4. var
  5.   C: TControl;
  6. begin
  7.    for C in Panel1.GetEnumeratorControls do
  8.      if C is TToggleBox then
  9.      begin
  10.        C.OnClick := nil;
  11.        TToggleBox(C).State := CStateWhenSender[Sender = C];
  12.        C.OnClick := @TglBoxClick;
  13.      end;
  14. end;

Arioch

  • Sr. Member
  • ****
  • Posts: 421
Re: Lazarus trank: stack overflow for new revisions
« Reply #14 on: September 26, 2022, 09:08:19 pm »
I suggest using modern language features:
Code: Pascal  [Select][+][-]
  1. const
  2.   CStateWhenSender: array [Boolean] of TCheckBoxState = (cbUnchecked, cbChecked);
  3. ...
  4.        TToggleBox(C).State := CStateWhenSender[Sender = C];
  5.  

...when you are paid per line :-)

Code: Pascal  [Select][+][-]
  1.    TToggleBox(C).Checked := Sender = C;
  2.  

...but the only thing he really needed was one line in .FormCreate:  TCheckBox.VCL_Onlick_Emulation := False;

However, it is trunk,, not release, so who knows what it all would end with.



The coide with OnClick's was fragile, by the way.

...so set TToggleBox.OnClick to nil when changing the state

Code: Pascal  [Select][+][-]
  1.     for i:= 0 to Pred(Panel1.ControlCount) do
  2.       if TControl(Panel1.Controls[i]).InheritsFrom(TToggleBox) then
  3.         begin
  4.           TToggleBox(Panel1.Controls[i]).OnClick:= nil;
  5.  
  6. +++ TRY
  7.  
  8.           if TToggleBox(Panel1.Controls[i]).Equals(TToggleBox(Sender))
  9.             then
  10.               TToggleBox(Panel1.Controls[i]).State:= cbChecked
  11.             else
  12.               TToggleBox(Panel1.Controls[i]).State:= cbUnchecked;
  13.  
  14.  
  15. +++ FINALLY
  16.  
  17.           TToggleBox(Panel1.Controls[i]).OnClick:= @TglBoxClick;
  18.  
  19. +++ END
  20.         end;
« Last Edit: September 26, 2022, 09:14:15 pm by Arioch »

 

TinyPortal © 2005-2018