Recent

Author Topic: Windows 10 and FormClose  (Read 6472 times)

Bart

  • Hero Member
  • *****
  • Posts: 3452
    • Bart en Mariska's Webstek
Re: Windows 10 and FormClose
« Reply #90 on: July 11, 2019, 04:09:31 pm »
and that its thread can be randomly overwritten. That process can also clear out its garbage by a subsequent write that is luckily the same or greater size and has compatible data.

What have threads got to do with this topic?
All writing to the string in this example happens in the same thread anyway.

Bart

Bart

  • Hero Member
  • *****
  • Posts: 3452
    • Bart en Mariska's Webstek
Re: Windows 10 and FormClose
« Reply #91 on: July 11, 2019, 04:54:57 pm »
Yes I can get cmdrpn2.zip to crash with fpc 3.0.4: 1<enter><alt-F4>
It won't crash with fpc trunk though.

Probably not related, but the "if TmpStr <> RegBox1.Text" in OnChange somehow triggers another OnChange, why's that?

The above sequence triggers 3 onchanges.
If I comment out the "if TmpStr <> RegBox1.Text" I only get 2 OnChanges

Bart

rvk

  • Hero Member
  • *****
  • Posts: 3743
Re: Windows 10 and FormClose
« Reply #92 on: July 11, 2019, 05:05:57 pm »
Probably not related, but the "if TmpStr <> RegBox1.Text" in OnChange somehow triggers another OnChange, why's that?

The above sequence triggers 3 onchanges.
If I comment out the "if TmpStr <> RegBox1.Text" I only get 2 OnChanges
That's also what I saw with "if Edit1.Text <> 'a' then ". It triggered the crash ending the program.
When commenting out that line (which factually does NOTHING) the crash went away.

I don't think the "if TmpStr <> RegBox1.Text" triggers an OnChange but there is some code overwritten with garbage (by Str()) which causes some code to crash (or jump to inexplicable places).

Bart

  • Hero Member
  • *****
  • Posts: 3452
    • Bart en Mariska's Webstek
Re: Windows 10 and FormClose
« Reply #93 on: July 11, 2019, 06:51:39 pm »
If you comment out the call to Str() in TAppForm.RegBox1Change then you only get 2 OnChanges with the sequence <1><enter><alt+F4>

Changing TmpStr inside TAppForm.RegBox1Change in any way (not using Str()) and then querying the content of RegBox1.Text will trigger a 3rd OnChange to occur.
This happens also if Icomment out all the calls to Str() inside the program.

Bart
« Last Edit: July 11, 2019, 07:01:18 pm by Bart »

rvk

  • Hero Member
  • *****
  • Posts: 3743
Re: Windows 10 and FormClose
« Reply #94 on: July 11, 2019, 10:58:02 pm »
Can you check out if Sender is the same all 3 times?

Or if the OnChange is really triggered by the Change() procedure (by overriding it and seeing if that is also called three times).

It's strange that even without Str() this happens. That could indeed mean it's some kind of reference count problem.

Bart

  • Hero Member
  • *****
  • Posts: 3452
    • Bart en Mariska's Webstek
Re: Windows 10 and FormClose
« Reply #95 on: July 12, 2019, 12:11:53 am »
Sender is always RegBox1.
If you make TmpStr a private var of TAppForm, then only 2 OnChanges occur.

Bart
« Last Edit: July 12, 2019, 12:17:06 am by Bart »

BrunoK

  • Full Member
  • ***
  • Posts: 138
  • Retired programmer
Re: Windows 10 and FormClose
« Reply #96 on: July 12, 2019, 10:35:57 am »
Looking at the generated code in assembler, my conclusion is that the compiler adds a <fpc_ansistr_decr_ref> before <fpc_ansistr_float> and that does a FreeMem  with not very clear consequences.

Proposed code to circumvent the problem :

Code: Pascal  [Select]
  1.  
  2. procedure TAppForm.RegBox1Change(Sender: TObject);
  3. var
  4.   lShortStr : shortstring;
  5. begin
  6.   Str(Reg1: 0: Dsp, lShortStr); // First to a shortstring
  7.   TmpStr := lShortStr;          // then assign it to the ansistring
  8.   if TmpStr <> RegBox1.Text then
  9.   begin
  10.     // the above if line causes error when exiting program
  11.   end;
  12. end;
  13.  
  14. procedure TAppForm.UpdDsp;
  15. var
  16.   lShortStr : shortstring;
  17. begin
  18.   Str(Reg1: 0: Dsp, lShortStr); // First to a shortstring
  19.   TmpStr := lShortStr;          // then assign it to the ansistring
  20.   RegBox1.Text := TmpStr;
  21. end;
  22.  

Lazarus trunk r. 59978/03.01.2019 (+/- patches regarding enabled, TScrollBar, TCursorImage). FPC 3.0.4 32 bits. (+heaptrc with leaked ClassName+Revisited TList) , Windows 10 Pro x64 (v. 1803)

rvk

  • Hero Member
  • *****
  • Posts: 3743
Re: Windows 10 and FormClose
« Reply #97 on: July 12, 2019, 11:07:00 am »
Just changing {$I+} to {$I-} fixes it too, with much more certainty to no more side-effects about ref-counting.

We can conclude there is something fishy about the combination of str() with decimals in FPC 3.0.4
Since it can't be reproduced in later version, there is a high probability it is fixed in those versions.
Using ShortString, {$H-} or intermediate variables can fix it temporarily in older versions.

The firing twice of the OnChange event lies in the TControl.RealSetText(const Value: TCaption);
Before reaching it, the Text should already have been changed by TWin32WSCustomEdit.SetText() but somehow the RealGetText = Value doesn't exit that procedure (hence the second firing of OnChange due to the CM_TEXTCHANGED). Maybe this is also due to the fact TmpStr was already freed and the wrong value was passed to the TEdit at some point.

But that also doesn't seem to happen in later versions anymore.

rick2691

  • Sr. Member
  • ****
  • Posts: 374
Re: Windows 10 and FormClose
« Reply #98 on: July 12, 2019, 01:26:11 pm »
@Bart

I new I was using the wrong word when I typed "thread," but I was meaning its "sequence of characters or data." Sorry for being ambiguous.

@rvk

Have you looked at the code... TmpStr:= FloatToStrF(Reg1,ffExponent,17,0);

Can FloatToStrF have the same issue as Str() has?
Windows 10, LAZ 1.6.4, FPC 3.0.2, SVN 54278, i386-win32-win32/win64, forms use windows unit

rick2691

  • Sr. Member
  • ****
  • Posts: 374
Re: Windows 10 and FormClose
« Reply #99 on: July 12, 2019, 02:23:05 pm »
Additionally... UpdDsp, UpdKey, UpdSto, Hms2Hrs, and Hrs2Hms use a decimal formatting Str()

FormCloseQuery uses Str() without decimal formatting.
Windows 10, LAZ 1.6.4, FPC 3.0.2, SVN 54278, i386-win32-win32/win64, forms use windows unit

wp

  • Hero Member
  • *****
  • Posts: 5831
Re: Windows 10 and FormClose
« Reply #100 on: July 12, 2019, 03:39:41 pm »
Can FloatToStrF have the same issue as Str() has?
I think so, it uses Str() with decimals internally. I have not seen them in the Format() function which does use str() but without the decimals parameter.
Lazarus trunk / fpc 3.0.4 / all 32-bit on Win-10

rvk

  • Hero Member
  • *****
  • Posts: 3743
Re: Windows 10 and FormClose
« Reply #101 on: July 12, 2019, 03:43:58 pm »
Because your program isn't really string-oriented (long strings etc.) I wouldn't take any chances and just change the {$I+} to {$I-} and be done with it. It's the safest way.

Otherwise move the TmpStr to local variables there where you need it. Using it as global variable (or even as class-property) will give the same problem (which is probably a reference-count problem surrounding str() and/or global string variables in fpc 3.0.4 and lower).

rick2691

  • Sr. Member
  • ****
  • Posts: 374
Re: Windows 10 and FormClose
« Reply #102 on: July 12, 2019, 04:26:12 pm »
I have already made TmpStr a local variable, and implemented the Str() to lshortstring as someone has suggested. It has been operating without fail ever since.

But in UpdDsp and UpdSto there are multiple writes to TmpStr. I don't see how it can be protected within the function block. It should be just as vulnerable with that situation.
Windows 10, LAZ 1.6.4, FPC 3.0.2, SVN 54278, i386-win32-win32/win64, forms use windows unit

rvk

  • Hero Member
  • *****
  • Posts: 3743
Re: Windows 10 and FormClose
« Reply #103 on: July 12, 2019, 04:53:16 pm »
I have already made TmpStr a local variable, and implemented the Str() to lshortstring as someone has suggested. It has been operating without fail ever since.
Like I said, if you used {$H-} you wouldn't need to use ShortString because all Strings are automatically ShortStrings (i.e. String[255]). So that is the safest way. You wouldn't need to adjust anything else and your program will be safe for the future too.

But in UpdDsp and UpdSto there are multiple writes to TmpStr. I don't see how it can be protected within the function block. It should be just as vulnerable with that situation.
Moving TmpStr to local (as LongString), you eliminate a lot of reference counting and using it (directly) in Str() wouldn't impact it as much. But with a lot of Str() with LongString, it could still go wrong (probably). But the chances of it happening are a lot less.

So my suggestion would be to not use TmpStr (as LongString) directly with Str().
So you could do Str(ShortString:x:y) and assign it later to a LongString (i.e. TmpStr := Result_ShortString_From_Str)
or
Just add {$H-} at the top (instead of {$H+} and be done with all the hassle of checking for Long- and ShortStrings altogether.

wp

  • Hero Member
  • *****
  • Posts: 5831
Re: Windows 10 and FormClose
« Reply #104 on: July 12, 2019, 05:21:00 pm »
Just add {$H-} at the top (instead of {$H+} and be done with all the hassle of checking for Long- and ShortStrings altogether.
This recommendation is probably ok for a calculator application, but not in general, IMHO. The user at least must be aware of the consequence that strings cannot be longer than 255 characters then.
Lazarus trunk / fpc 3.0.4 / all 32-bit on Win-10