mORMot Open Source

macfly

Member

From: Brasil

Registered: 2016-08-20

Posts: 374

[Solved] Strange behavior in ComputeFieldsBeforeWrite

I started having this error today, but looking for older versions including 2019 this method has always been like this.

So I don't know if it's a bug or an error on my side.

My TSQLRecord has fields of type TCreateTime and TModTime.

And the ComputeFieldsBeforeWrite method responsible for filling these fields, has this code:

 if integer(types)<>0 then begin
        i64 := aRest.ServerTimestamp;
        for F := 0 to Fields.Count-1 do
        with TSQLPropInfoRTTIInt64(Fields.List[f]) do //<-- this causes invalid type cast exception
        if SQLFieldType in types then
          fPropInfo.SetInt64Prop(Self,i64);
      end;  

The looping is being done in all fields. So when it arrives in a field that is not an integer, the error occurs.
I believe a type check should be done on this part.

However, I don't understand why I didn't have this error before, since I always used these fields.

Last edited by macfly (2020-04-26 14:20:01)

macfly

Member

From: Brasil

Registered: 2016-08-20

Posts: 374

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

Confirming that doing the type check solves the problem. But I still don't understand why this error didn't happen before.

unit mormot.pas line 32295

 if integer(types)<>0 then begin
        i64 := aRest.ServerTimestamp;
        for F := 0 to Fields.Count-1 do
        if Fields.List[f].InheritsFrom(TSQLPropInfoRTTIInt64) then //<-- added type check
          with TSQLPropInfoRTTIInt64(Fields.List[f]) do
          if SQLFieldType in types then
            fPropInfo.SetInt64Prop(Self,i64);
      end;  

Last edited by macfly (2020-04-25 18:01:37)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,660

Website

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

I don't get it.
The SQLFieldType property if part of TSQLPropInfo, so you can always access it.
And "types" set can contain ONLY sftModTime and sftCreateTime, which are implemented with ONLY TSQLPropInfoRTTITimeLog, which is ALWAYS a TSQLPropInfoRTTIInt64 class.
So I see NO reason why there is an error here.

What is "the error" which occurs in your case?
The TSQLPropInfoRTTIInt64(Fields.List[f]) expression makes a strict typecast with no check at runtime, and should never raise any exception. It is just a pointer access in the generated asm.
Unless you changed the default compilations options. Which is very likely to break in a lot of place in the framework.
Did you change something in Synopse.inc ?

The framework code makes a lot of similar assumptions, when it is sure that we can force the typecast.
If we write

with (Fields.List[f] as TSQLPropInfoRTTIInt64) do 

or

if Fields.List[f].InheritsFrom(TSQLPropInfoRTTIInt64) then
  with TSQLPropInfoRTTIInt64(Fields.List[f]) do

the code will call some internal RTL function, which slows down the execution.
It may sound like premature optimization, but we made sure that there is no problem.

macfly

Member

From: Brasil

Registered: 2016-08-20

Posts: 374

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

Thanks for  the explanations.

The error is RunError(219)
EInvalidCast / Invalid Type Cast

A remind that Lazarus has Freezed in a Debug Session and I had to finish via Task Manager.

I believe it must have corrupted the project settings, although everything is apparently OK.

I disabled "Verify Method Calls" that add -CR to compiler options and the error whas gone.

But this option is standard when creating the build mode for Debug.

I will keep looking for the reason why this error started to occur.

macfly

Member

From: Brasil

Registered: 2016-08-20

Posts: 374

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

Hi, @ab, Looking at the code better, the looping through the fields will always be executed if there are one  sftModTime and or sftCreateTime field in TSQLRecord.

I created a simple project just to reproduce and this error occurs in both Lazarus + FPC3.3.1 / Lazarus + FPC3.2.

So I believe it is not a problem with file corruption. I downloaded mORMOt again too, and tested it with older versions.

See the example below, at some point it will try to cast the UserName field which is TSQLPropInfoRTTIRawUTF8  in TSQLPropInfoRTTIInt64

  TSQLRecordTest = class(TSQLRecord)
  private
  published
    property CreatedAt: TCreateTime read fCreatedAt write fCreatedAt;
    property ModifiedAt: TModTime read fModifiedAt write fModifiedAt;
    property UserName: RawUTF8 read FUserName write FUserName;
  end; 

 if integer(types)<>0 then begin // <-- if are any  sftModTime and or sftCreateTime field this is true
        i64 := aRest.ServerTimestamp;
        for F := 0 to Fields.Count-1 do  // <-- loop in ALL Fields
          with TSQLPropInfoRTTIInt64(Fields.List[f]) do  //<-- Try to typecast UserName field (TSQLPropInfoRTTIRawUTF8 )  in TSQLPropInfoRTTIInt64
          if SQLFieldType in types then
            fPropInfo.SetInt64Prop(Self,i64);
      end;  

Last edited by macfly (2020-04-25 21:43:14)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,660

Website

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

You should just NOT define "Verify Method Calls" in FPC.

Sadly, we can't disable it (IIRC) by a compilation directive.

It is breaking compatibility with Delphi, for sure, where such thing doesn't exist.

I admit that it is a problem in a "pure FPC" world.
I will try to fix it for mORMot2 - since I am removing all hints and warnings on FPC with the rewritten code base.

macfly

Member

From: Brasil

Registered: 2016-08-20

Posts: 374

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

Thanks for the attention as always.

I suspected that. Because nothing has been changed.

What I don't understand is because he didn't take this before, since this option is active by default when creating a debug build.

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,660

Website

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

I tried to fix what I found, without touching the performance.

Please check https://synopse.info/fossil/info/36a07b26ee

macfly

Member

From: Brasil

Registered: 2016-08-20

Posts: 374

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

Thanks @ab,

Has solved.

I will leave this option active.

If occurs in any other location  I will inform you. For Mormot2.

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,660

Website

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

Great!
Thanks!

macfly

Member

From: Brasil

Registered: 2016-08-20

Posts: 374

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

For mORMot 2

Another place where the compiler raises an exception (with Verify Method Calls ON)

mormot.pas
line 32210
method TSQLRecord.Validate

//Adding both filter and validator
TSQLTest.AddFilterOrValidate('Name', TSynFilterUpperCaseU.Create());
TSQLTest.AddFilterOrValidate('Name', TValidateName.Create());  

Attempt to cast a TSynFilter in TSynValidate.

function TSQLRecord.Validate
..
    for i := 0 to length(Filters[f])-1 do begin
      Validate := TSynValidate(Filters[f,i]); //<-- here
      if Validate.InheritsFrom(TSynValidate) then begin
        if Value='' then

I believe that doing the inheritance check before will not affect performance, and you should avoid this error.

function TSQLRecord.Validate(...)
..
    for i := 0 to length(Filters[f])-1 do begin
      if Filters[f,i].InheritsFrom(TSynValidate) then begin
        Validate := TSynValidate();

Note: In the Filter method it is already done this way ...

function TSQLRecord.Filter(...)
...
if Filters[f,i].InheritsFrom(TSynFilter) then begin 

Last edited by macfly (2020-05-05 19:49:26)

macfly

Member

From: Brasil

Registered: 2016-08-20

Posts: 374

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

Hi @ab,

If some applicable methods uses "out" instead of "var", lazarus would stop showing many unnecessary hints.

var
  rec : TMyRecord;
...
  RecordZero(rec, TypeInfo(TMyRec));

Hint: Local variable "rec" of a managed type does not seem to be initialized

If the RecordZero uses out to pass the parameter, Lazarus does not display this hint.

mpv

Member

From: Ukraine

Registered: 2012-03-24

Posts: 1,571

Website

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

Passing management type using out adds additional asm code, so sometimes var is ok

macfly

Member

From: Brasil

Registered: 2016-08-20

Posts: 374

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

The "out" forces compiler to reset the variable, correct?

I didn't really think about the performance issue.

In this case it is better not to change.

{%H-} supress this hint

 RecordZero(rec{%H-}, TypeInfo(TMyRec));

mpv

Member

From: Ukraine

Registered: 2012-03-24

Posts: 1,571

Website

Re: [Solved] Strange behavior in ComputeFieldsBeforeWrite

The "out" forces compiler to reset the variable, correct?

I didn't really think about the performance issue.

In this case it is better not to change.

{%H-} supress this hint

 RecordZero(rec{%H-}, TypeInfo(TMyRec));

Yes, compiler will reset the variable for strings, records etc.

@ab, I also vote for {%H-} in mORMot2. I know, you don't like this directive (you remove it from some of my merge requests), but having too many warnings in compiler output sometimes hide a real problems.

Last edited by mpv (2020-05-07 18:18:31)