mORMot Open Source

EgonHugeist

Member

From: Germany

Registered: 2013-02-15

Posts: 190

dangling String RefCounts in TDocVariant.VName

Hi Arnaud,

in some circumstances i've some reports of leaking ansistring in my projects. I remember there was a forum discussion about.. But i can't find it any more.
I think the reports and my newly issue having same reasons.

i've a little test case for you:

function ShowMyMissingName: RawJSON;
var
  Item, Arr: Variant;
begin
  Arr := SynCommons._Arr([]);
  Item := _JSON('{"ID": 1,"Notation": "ABC"}');
  Item.Add('Price', 10.10);
  Item.Add('CustomNotation', 'XYZ');
  Arr.Add(Item);
  Item.Delete('Price');
  Arr.Add(Item);
  Result := VariantSaveJSON(Arr);
end;

While tracing the values in the Arr-Variant you'll find a missing Name of "Price" in Element[0] of the JSON wheras the Value still exists.
Is this expected? Did i forgot to set an option?

looking to

procedure TDocVariant.CopyByValue(var Dest: TVarData; const Source: TVarData);
var S: TDocVariantData absolute Source;
    D: TDocVariantData absolute Dest;
    i: integer;
begin
  //Assert(Source.VType=DocVariantVType);
  if Dest.VType and VTYPE_STATIC<>0 then
    VarClear(variant(Dest)); // Dest may be a complex type
  D.VType := S.VType;
  D.VOptions := S.VOptions;
  D.VKind := S.VKind;
  D.VCount := S.VCount;
  pointer(D.VName) := nil; // avoid GPF
  pointer(D.VValue) := nil;
  if S.VCount=0 then
    exit; // no data to copy
  D.VName := S.VName; // names can always be safely copied
  // slower but safe by-value copy
  SetLength(D.VValue,S.VCount);
  for i := 0 to S.VCount-1 do
    D.VValue[i] := S.VValue[i];
end;

you assing the D.VName from S.VName which does increment the array refcount but not the string refcounts..

Some reason for this behavior:

function TDocVariantData.Delete(Index: integer): boolean;
begin
  if cardinal(Index)>=cardinal(VCount) then
    result := false else begin
    dec(VCount);
    if VName<>nil then
      VName[Index] := ''; //EH: check the refcount ... before doing this and after -> nothing changed even if refcount > 1 
    VarClear(VValue[Index]);
    if Index<VCount then begin
      if VName<>nil then begin
        MoveFast(VName[Index+1],VName[Index],(VCount-Index)*sizeof(Pointer));
        NativeUInt(VName[VCount]) := 0; // avoid GPF
      end;
      MoveFast(VValue[Index+1],VValue[Index],(VCount-Index)*sizeof(variant));
      TVarData(VValue[VCount]).VType := varEmpty; // avoid GPF
    end;
    result := true;
  end;
end;

For me it seem's you start from the premisse on setting

VName[Index] := ";

the compiler checks the DynArr refcount and copies the array on changing a element. But iv'e noticed this doesn't happen. Check the RefCount by

PLongInt(NativeUInt(VName)-8)^ = x;

and the String-Refcounts of course.

Exchanging your assignment in TDocVariant.CopyByValue by:

  // slower but safe by-value copy
  SetLength(D.VName,Length(S.VName));
  for i := 0 to High(S.VName) do
    if Pointer(S.VName[i]) <> nil then 
      D.VName[i] := S.VName[i] else
      Break; 

resolves my problem. The compiler makes a unique array and increments the stringRefcounts like expected.

What do you think?

Edit:
I know you want a fast copy by ref but this should be optional if dvoValueCopiedByReference is set. Am i wrong?

Cheers, Michael

Last edited by EgonHugeist (2017-08-16 17:12:01)

EgonHugeist

Member

From: Germany

Registered: 2013-02-15

Posts: 190

Re: dangling String RefCounts in TDocVariant.VName

ps.
To successfully reproduce the memoryleak of an ansistring just add

Arr._(0).Add('Test', 10);

@end of my testcase

Edit: Weird, can't reproduce the leak any more after starting the IDE again. But my report about a missing fieldname remains.

Last edited by EgonHugeist (2017-08-15 20:36:45)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,659

Website

Re: dangling String RefCounts in TDocVariant.VName

This is indeed confusing, but as expected.
You are assigning twice the same Item.
Once added, the item is expected to be owned by the array.
This is why you have a leak.
Make a copy of Item before Arr.Add(), e.g. by using _Unique() - see https://synopse.info/files/html/Synopse … l#TITLE_43

EgonHugeist

Member

From: Germany

Registered: 2013-02-15

Posts: 190

Re: dangling String RefCounts in TDocVariant.VName

Arnaud, i wouldn't bother you if there is a solution.
It is confusing becuase changing the TestCase to:

function ShowMyMissingName: RawJSON;
var
  Item, Arr: Variant;
begin
  Arr := SynCommons._Arr([]);
  Item := _JSON('{"ID": 1,"Notation": "ABC"}');
  Item.Add('Price', 10.10);
  Item.Add('CustomNotation', 'XYZ');
  Arr.Add(_Copy(Item)); // <------------------with _Copy()
  Item.Delete('Price');
  Arr.Add(Item);
  Result := VariantSaveJSON(Arr);
end;

doesn't help neither.

Ok second testcase 4 you (done everything double with _Copy() and _Unique() )

function ShowMyMissingName: RawJSON;
  Item, Item2, Item3, Arr: Variant;
begin
  Arr := SynCommons._Arr([]);
  Item := _JSON('{"ID": 1,"Notation": "ABC", "Price": 10.1, "CustomNotation": "XYZ"}');
  Arr.Add(Item);
  _Unique(Item); //as documented in [url]https://synopse.info/files/html/Synopse[/url] … l#TITLE_43
  Item.Delete('Price'); //this still effects element 0 in the array because you still refrence same VName array
  Item2 := _Copy(Item); //as you can see i do copy not by ref
  _Unique(Item2); //now i also make the item unique which is redundant because copy is doing this already
  Item2.ID := 2;
  Arr.Add(Item2); //i add second item to the array

  Item3 := _Copy(Arr._(1)); //copy second element of Arr into Item3 (currently not assigned)
  _Unique(Item3); //make item3 unique which is redundant too
  Item3.Delete('CustomNotation'); //after doing this step we'll have a json parse error for element[0] in the array
  Item3.ID := 3;
  Arr.Add(Item3); //add item3 to array as third element

  Result := VariantSaveJSON(Arr);
end;

Result is:

[{
    "ID": 1,
    "Notation": "ABC",
    "": 10.1,
    "": "XYZ"
}, {
    "ID": 2,
    "Notation": "ABC",
    "": "XYZ"
}, {
    "ID": 3,
    "Notation": "ABC"
}]

Done like documented, thought. But wat was happen with my array??

Third case 4 you:

 
function ShowMyMissingName: RawJSON;
begin
  Arr := SynCommons._Arr([_JSON('{"ID": 1,"Notation": "ABC", "Price": 10.1, "CustomNotation": "XYZ"}'), _JSON('{"ID": 2,"Notation": "XYZ", "Price": 9.1, "CustomNotation": "aaaa"}')]);
  Item := _Copy(arr._(0));
  _Unique(Item1); //make item1 unique which is redundant because of _Copy()
  Item.Delete('CustomNotation');
  Result := VariantSaveJSON(Arr);
end;

Result is:

[{
    "ID": 1,
    "Notation": "ABC",
    "Price": 10.1,
    "": "XYZ"
}, {
    "ID": 2,
    "Notation": "XYZ",
    "Price": 9.1,
    "CustomNotation": "aaaa"
}]

Done like documented too. Is this realy expected?

I know this case isn't usual.
However what do you think about this:

function TDocVariantData.Delete(Index: integer): boolean;
const RefCountOffSet = SizeOf(SizeInt)+SizeOf(Longint); //<-  depends to compiler and target (just a proposal)
var N: TRawUTF8DynArray;
  I: Integer;
begin
  if cardinal(Index)>=cardinal(VCount) then
    result := false else begin
    dec(VCount);
    if VName<>nil then begin
      If PLongInt(NativeUInt(VName)-RefCountOffSet)^ > 1 then begin
        System.SetLength(N, Length(VName)); //<- make a unique dynarr
        for i := 0 to length(VName) do
          if (Pointer(VName[i]) <> nil) or ((I < High(VName)) and (Pointer(VName[i+1]) <> nil)) then //two empty names lead to parsing errors 
            N[i] := VName[i] else
            Break;
        VName := N;
      end; 
      VName[Index] := '';
    end;
    VarClear(VValue[Index]);
    if Index<VCount then begin
      if VName<>nil then begin
        MoveFast(VName[Index+1],VName[Index],(VCount-Index)*sizeof(pointer));
        PtrUInt(VName[VCount]) := 0; // avoid GPF
      end;
      MoveFast(VValue[Index+1],VValue[Index],(VCount-Index)*sizeof(variant));
      TVarData(VValue[VCount]).VType := varEmpty; // avoid GPF
    end;
    result := true;
  end;
end;

This would reduce the memallocs to minimum but there are the offsets and type of RefCount ... which may differ for compiler + target..

else ....
The only code to work with would be something like:

Item := _JSON(VariantSaveJSON(arr._(0))

Should i go this route?

Last edited by EgonHugeist (2017-08-16 17:43:30)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,659

Website

Re: dangling String RefCounts in TDocVariant.VName

Try with plain TDocVariantData content, without any late binding.

EgonHugeist

Member

From: Germany

Registered: 2013-02-15

Posts: 190

Re: dangling String RefCounts in TDocVariant.VName

Don't know why you suggest it. Minimal simple case:

function ShowMyMissingName: RawJSON;
var
  Item: Variant;
  Item2: TDocVariantData;
begin
  Item := _JSON('{"ID": 1,"Notation": "ABC", "Price": 10.1, "CustomNotation": "XYZ"}');
  Item2.InitCopy(Item, []);
  Item2.I['ID'] := 2;
  Item2.Delete(Item2.GetValueIndex('CustomNotation'));
  Result := VariantSaveJSON(Item);
end;

Result is:

{
    "ID": 1,
    "Notation": "ABC",
    "Price": 10.1,
    "": "XYZ"
}

You've a inconsistency because all copies of the origin do point to the same VName array. There is no differenz between late binding or plain TDocVariant.
Each origin will be invalid if i delete fields from the copy or the copy of a copy etc.

Funny is if you add now a new field+Value to Item2 like:

function ShowMyMissingName: RawJSON;
var
  Item: Variant;
  Item2: TDocVariantData;
begin
  Item := _JSON('{"ID": 1,"Notation": "ABC", "Price": 10.1, "CustomNotation": "XYZ"}');
  Item2.InitCopy(Item, []);
  Item2.I['ID'] := 2;
  Item2.Delete(Item2.GetValueIndex('CustomNotation'));
  Item2.AddValue('mORMot', 100);
  Result := VariantSaveJSON(Item);
end;

the original item has a new name for value "XYZ":

{
    "ID": 1,
    "Notation": "ABC",
    "Price": 10.1,
    "mORMot": "XYZ"
}

where i would expect "CustomNotation".

Last edited by EgonHugeist (2017-08-17 17:42:47)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,659

Website

Re: dangling String RefCounts in TDocVariant.VName

Please try https://synopse.info/fossil/info/a38e20758c

EgonHugeist

Member

From: Germany

Registered: 2013-02-15

Posts: 190

Re: dangling String RefCounts in TDocVariant.VName

Works like a charm! Thanks Arnaud.