mORMot Open Source

bigstar

Member

Registered: 2014-01-31

Posts: 5

Possible bug in SynCommons.pas TRawUTF8List.BeginUpdate;

Hello,

While experimenting with TRawUTF8List I noticed that BeginUpdate/EndUpdate didn't work as expected.

Upon reviewing the code the logic seems flawed.

procedure TRawUTF8List.BeginUpdate;
begin
  inc(fOnChangeLevel);
  if fOnChangeLevel>0 then
    exit;
  fOnChangeHidden := fOnChange;
  fOnChange := OnChangeHidden;
  fOnChangeTrigerred := false;
end;

The first call to BeginUpdate will increase fOnChangeLevel from 0 to 1 and the next line evaluates if fOnChangeLevel > 0 then exit. The last 3 lines will never be executed.

Last edited by bigstar (2014-12-18 16:06:57)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,740

Website

Re: Possible bug in SynCommons.pas TRawUTF8List.BeginUpdate;

Sounds just fine to me.
The call count allows to handle nested BeginUpdate/EndUpdate calls so that the OnChange event would not be called during the process.

bigstar

Member

Registered: 2014-01-31

Posts: 5

Re: Possible bug in SynCommons.pas TRawUTF8List.BeginUpdate;

Take a look at this quick demo to illustrate the bug

var
    Counter: Integer = 0;

procedure TForm1.DoOnChange(Sender: TObject);
begin
     Counter := Counter + 1;
     Form1.Caption := IntToStr(Counter);
end;

procedure TForm1.FormCreate(Sender: TObject);
var
   list: TRawUTF8List;
begin
     Counter := 0;
     list := TRawUTF8List.Create(false);

     list.OnChange := Form1.DoOnChange;

     list.BeginUpdate;
     list.Add('1');
     list.Add('2');
     list.Add('3');
     list.Add('4');
     list.EndUpdate;

     list.Free;
end;

How many times do you think DoOnChange() is triggered? It's not 1 but 4 times, one for each .Add()

I corrected the problem by changing the evaluation from >0 to >1 as shown below, not sure if this is the appropriate fix, if anything maybe a comment is needed.

procedure TRawUTF8List.BeginUpdate;
begin
  inc(fOnChangeLevel);
  if fOnChangeLevel>1 then
    exit;
  fOnChangeHidden := fOnChange;
  fOnChange := OnChangeHidden;
  fOnChangeTrigerred := false;
end;

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,740

Website

Re: Possible bug in SynCommons.pas TRawUTF8List.BeginUpdate;

Oups... you are right!

Should be fixed by http://synopse.info/fossil/info/738101df0d