Is delphi TQueue buggy? Using TQueue return nil with dequeue

Question

I don't understand why this very simple code failed? I'm on Delphi Tokyo release 2.

{$APPTYPE CONSOLE}

uses
  System.SysUtils,
  System.Generics.Collections;

procedure Main;
var
  aQueue: TQueue<TBytes>;
  aBytes: TBytes;
begin
  aQueue := TQueue<TBytes>.create;

  aBytes := TEncoding.UTF8.GetBytes('abcd');
  aQueue.Enqueue(aBytes);
  aBytes := aQueue.Dequeue;
  Writeln(Length(aBytes)); // outputs 4 as expected

  aBytes := TEncoding.UTF8.GetBytes('abcd');
  aQueue.Enqueue(aBytes);
  aBytes := aQueue.Dequeue;
  Writeln(Length(aBytes)); // outputs 0
end;

begin
  Main;
  Readln;
end.

Is this a bug?

NOTE: The code works correctly on XE4, but fails also on Berlin.

Answer 1

This is indeed a bug. The code works correctly in XE7, but not XE8. In XE8 the output is 0 for both attempts.

Certainly the XE8 generic collections were very buggy and subsequent releases fixed many of the defects. Clearly not all have been fixed.

In XE8 Embarcadero attempted to address the issue of generic bloat caused by weaknesses in their compile/link model. Unfortunately, instead of tackling the problem at the root, they chose instead to address the issue in the library code for generic collections. Doing so they completely broke many of the generic collection classes, proving that their unit testing was weak. And of course, by addressing the problem this way they failed to address the issue of generic bloat for classes other than those in the generic collections. All in all, a sorry story that is seemingly still not over.

loki has just submitted a bug report: RSP-20400.

Note that this bug report is incorrect because (at least according to Stefan Glienke) the bug has been fixed in Tokyo 10.2.3. So upgrading to 10.2.3 should be the simplest way to resolve the problem.

Perhaps this bug report is more appropriate: RSP-17728.

Writing a generic queue isn't even difficult. Here's one that is known to work:

type
  TQueue<T> = class
  private
    FItems: TArray<T>;
    FCount: Integer;
    FFront: Integer;
  private
    function Extract(Index: Integer): T; inline;
    function GetBack: Integer; inline;
    property Back: Integer read GetBack;
    property Front: Integer read FFront;
    procedure Grow;
    procedure RetreatFront; inline;
  public
    property Count: Integer read FCount;
    procedure Clear;
    procedure Enqueue(const Value: T);
    function Dequeue: T;
    function Peek: T;
  public
    type
      TEnumerator = record
      private
        FCollection: TQueue<T>;
        FCount: Integer;
        FCapacity: Integer;
        FIndex: Integer;
        FStartIndex: Integer;
      public
        class function New(Collection: TQueue<T>): TEnumerator; static;
        function GetCurrent: T;
        property Current: T read GetCurrent;
        function MoveNext: Boolean;
      end;
  public
    function GetEnumerator: TEnumerator;
  end;

function GrownCapacity(OldCapacity: Integer): Integer;
var
  Delta: Integer;
begin
  if OldCapacity>64 then begin
    Delta := OldCapacity div 4
  end else if OldCapacity>8 then begin
    Delta := 16
  end else begin
    Delta := 4;
  end;
  Result := OldCapacity + Delta;
end;

{ TQueue<T> }

function TQueue<T>.Extract(Index: Integer): T;
begin
  Result := FItems[Index];
  if IsManagedType(T) then begin
    Finalize(FItems[Index]);
  end;
end;

function TQueue<T>.GetBack: Integer;
begin
  Result := Front + Count - 1;
  if Result>high(FItems) then begin
    dec(Result, Length(FItems));
  end;
end;

procedure TQueue<T>.Grow;
var
  Index: Integer;
  Value: T;
  Capacity: Integer;
  NewItems: TArray<T>;
begin
  Capacity := Length(FItems);
  if Count=Capacity then begin
    SetLength(NewItems, GrownCapacity(Capacity));
    Index := 0;
    for Value in Self do begin
      NewItems[Index] := Value;
      inc(Index);
    end;

    FItems := NewItems;
    FFront := 0;
  end;
end;

procedure TQueue<T>.RetreatFront;
begin
  inc(FFront);
  if FFront=Length(FItems) then begin
    FFront := 0;
  end;
end;

procedure TQueue<T>.Clear;
begin
  FItems := nil;
  FCount := 0;
end;

procedure TQueue<T>.Enqueue(const Value: T);
begin
  Grow;
  inc(FCount);
  FItems[Back] := Value;
end;

function TQueue<T>.Dequeue: T;
var
  Index: Integer;
begin
  Assert(Count>0);
  Result := Extract(Front);
  RetreatFront;
  dec(FCount);
end;

function TQueue<T>.Peek: T;
begin
  Assert(Count>0);
  Result := FItems[Front];
end;

function TQueue<T>.GetEnumerator: TEnumerator;
begin
  Result := TEnumerator.New(Self);
end;

{ TQueue<T>.TEnumerator }

class function TQueue<T>.TEnumerator.New(Collection: TQueue<T>): TEnumerator;
begin
  Result.FCollection := Collection;
  Result.FCount := Collection.Count;
  Result.FCapacity := Length(Collection.FItems);
  Result.FIndex := -1;
  Result.FStartIndex := Collection.Front;
end;

function TQueue<T>.TEnumerator.GetCurrent: T;
var
  ActualIndex: Integer;
begin
  ActualIndex := (FStartIndex + FIndex) mod FCapacity;
  Result := FCollection.FItems[ActualIndex];
end;

function TQueue<T>.TEnumerator.MoveNext: Boolean;
begin
  inc(FIndex);
  Result := FIndex<FCount;
end;

Answer 2

To add to David's answer, the bug is in the Enqueue method. The top branch should be handling all reference counted managed types.

  if IsManagedType(T) then
    if (SizeOf(T) = SizeOf(Pointer)) and (GetTypeKind(T) <> tkRecord) then
      FQueueHelper.InternalEnqueueMRef(Value, GetTypeKind(T))
    else
      FQueueHelper.InternalEnqueueManaged(Value)
  else

But here we see that dynamic arrays are conspicuously missing in InternalEnqueueMref, which falls through without doing anything:

procedure TQueueHelper.InternalEnqueueMRef(const Value; Kind: TTypeKind);
begin
  case Kind of
    TTypeKind.tkUString: InternalEnqueueString(Value);
    TTypeKind.tkInterface: InternalEnqueueInterface(Value);
{$IF not Defined(NEXTGEN)}
    TTypeKind.tkLString: InternalEnqueueAnsiString(Value);
    TTypeKind.tkWString: InternalEnqueueWideString(Value);
{$ENDIF}
{$IF Defined(AUTOREFCOUNT)}
    TTypeKind.tkClass: InternalEnqueueObject(Value);
{$ENDIF}
  end;
end;

It's so egregious, in fact, that the compiler actually produces no code for Enqueue when compiled (other than preamble) since the futility of the exercise can be determined from the types at compile time.

Project1.dpr.15: aQueue.Enqueue(aBytes);
0043E19E 8B45F8           mov eax,[ebp-$08]
0043E1A1 8945F4           mov [ebp-$0c],eax
0043E1A4 8B45FC           mov eax,[ebp-$04]
0043E1A7 83C008           add eax,$08
0043E1AA 8945F0           mov [ebp-$10],eax
Project1.dpr.16: aBytes := aQueue.Dequeue;
0043E1AD 8D45EC           lea eax,[ebp-$14]

This bug, therefore, would be expected to affect TQueue<T> for T being any type of dynamic array.