How to work around Delphi 10's bug with TList<_AnyDynamicArrays_>?
Asked Answered
C

1

9

I stumbled upon a bug in Delphi 10 Seattle Update 1. Lets take the following code :

procedure TForm1.Button1Click(Sender: TObject);
begin
//----------We crash here----------------
  FList.Items[0] := SplitString('H:E', ':');
end;

procedure TForm1.FormCreate(Sender: TObject);
begin
  FList := TList<TStringDynArray>.Create;
  FList.Add(SplitString('H:E', ':'));
  FList.Items[0] := SplitString('H:E', ':');
end;

At first glance, it would appear that TList<T> doesn't properly manage the lifetime of the dynamic arrays it contains, but then again, it works just fine if compiled in 64 bits, it only crash in 32 bits (I understand it doesn't mean the bug isn't present in 64 bits...).

Note that SplitString was used because if was the first function returning a dynamic array that came to my mind. The original problem was encountered with TList<TBookmark> which exhibits the same problem.

It is possible to work around the bug rewriting the procedure Button1Click like this :

procedure TForm1.Button1Click(Sender: TObject);
var MyArray : TStringDynArray;
begin
  MyArray := FList.Items[0];
  FList.Items[0] := SplitString('H:E', ':');
  //----------Yeah! We don't crash anymore!-----------
end;

But going around all my applications modifying them to work around this bug would not really be my prefered option. I'd much prefer find the offending routine and patch it in-memory if possible.

If anyone encountered this problem and found a workaround, I'd be grateful. Otherwise, I'll post mine when/if I find a proper workaround.

Also, please comment if the problem is still present in Berlin.

Camala answered 8/12, 2016 at 19:51 Comment(2)
Same error in Berlin Upd2.Er
From reported bugs with generic TList, it seems to be a minefield.Er
C
8

After all, the bug was still there in 64 bits. It didn't crash for TStringDynArray, but it did for other dynamic array types.

The source of the problem is found in the following code in Generics.Collections :

procedure TListHelper.DoSetItemDynArray(const Value; AIndex: Integer);
type
  PBytes = ^TBytes;
var
  OldItem: Pointer;
begin
  OldItem := nil;
  try
    CheckItemRangeInline(AIndex);

    TBytes(OldItem) := PBytes(FItems^)[AIndex];
    PBytes(FItems^)[AIndex] := TBytes(Value);

    FNotify(OldItem, cnRemoved);
    FNotify(Value, cnAdded);
  finally
    DynArrayClear(OldItem, FTypeInfo); //Bug is here.
  end;
end;

What happen is, the wrong TypeInfo is passed to DynArrayClear. In the case of a TList<TStringDynArray>, TypeInfo(TArray<TStringDynArray>) is passed instead of TypeInfo(TStringDynArray). From what I can tell, the proper call is:

DynArrayClear(OldItem, pDynArrayTypeInfo(NativeInt(FTypeInfo) + pDynArrayTypeInfo(FTypeInfo).Name).elType^);

The procedure being private makes it complicated to intercept. I did so using the fact that record helper can still access the private section of records in Delphi 10. I guess it will be more complicated for Berlin's users.

function TMyHelper.GetDoSetItemDynArrayAddr: TDoSetItemDynArrayProc;
begin
  Result := Self.DoSetItemDynArray;
end;

Hopefully, Embarcadero will fix it someday...

Camala answered 8/12, 2016 at 21:43 Comment(5)
pDynArrayTypeInfo(NativeInt(FTypeInfo) + pDynArrayTypeInfo(FTypeInfo).Name).elType^ is what you get if you use ElType.Er
Reported: RSP-16511: Crash in TListHelper.DoSetItemDynArray() due to wrong TypeInfo used to clear the old dynamic arrayBirdseed
It's almost as if they didn't write unit tests for every special case in this helper........Notions
@LURD if you mean I can just use pDynArrayTypeInfo(FTypeInfo).elType^, no I can't. While pDynArrayTypeInfo(FTypeInfo).Name is defined as a byte, it's implemented as a shortstring, as evidenced by the code in _DynArrayClear.Camala
ElType is a property in TListHelper. The getter is defined as: function TListHelper.GetElType: Pointer; begin Result := PDynArrayTypeInfo(PByte(FTypeInfo) + PDynArrayTypeInfo(FTypeInfo).name).elType^; end; Just use DynArrayClear(OldItem,ElType);Er

© 2022 - 2024 — McMap. All rights reserved.