TryGetvalue doesn't work if individual items are freed - memory leaks if they aren't

Question

I discovered that the Win32 compile of my FMX application has a bug!

The problem is simply that the TryGetValue returns unknown myPOi's if I {$define FREE_MYPOI}. If I don't define it (ie. not free each instance of myPOI after it is added to the dictionary), then Eurekalog reports memory leaks when the app exits (my POI hasn't been freed obviously).

Most interesting, is that this problem doesn't occur in the Android version of the code with FREE_MYPOI defined. (thankfully)

Is my way of adding items to the Dictionary correct? (for Android and Win32 FMX)

Here is the core code:

type
  TMyPOI = class(TObject)
  public
    Value: Integer;
    Timestamp: TDateTime;
  end;

...

function TForm2.CreateOrUpdate(username: String; NewTimestamp: TDateTime): String;
var
  poiTimeStamp: TDateTime;
  myPOI: TMyPOI;
  Index: Integer;
begin

  if PoiDict.TryGetValue(username, myPOI) then
  begin
    // existing POI
    Result := InttoStr(myPOI.Value) + ' ' + DateTimeToStr(myPoi.Timestamp);
    poiTimestamp := myPOI.Timestamp;
    // update the Poi's timestamp
    myPOI.Timestamp := NewTimeStamp;
    PoiDict.AddOrSetValue(username, myPOI);
  end
  else
  begin
    // add a new POI
    Index := Random(999);
    Result := IntToStr(Index) + ' ' + DateTimeToStr(NewTimeStamp);
    myPOI := TMyPOI.Create;
{$ifdef FREE_MYPOI}
    try
{$endif}
      myPOI.Value := Index;
      myPOI.Timestamp := NewTimeStamp;
      PoiDict.Add(username, myPOI);
{$ifdef FREE_MYPOI}
    finally
      myPOI.Free;
    end;
{$endif}
  end;

end;

initialization
  PoiDict := TDictionary<string, TMyPOI>.Create;

finalization
  PoiDict.Free;

end.

Addendum: This is not an ARC-specific question. It is a question about managing object references.

Answer 1

First, don't ever Free any object you just created and added to any kind of collection. You should transfer ownership over that object to that collection. Following code is inherently broken on Windows - because you are creating dangling references inside dictionary - they will blow on you sooner or later.

    myPOI := TMyPOI.Create;
{$ifdef FREE_MYPOI}
    try
{$endif}
      myPOI.Value := Index;
      myPOI.Timestamp := NewTimeStamp;
      PoiDict.Add(username, myPOI);
{$ifdef FREE_MYPOI}
    finally
      myPOI.Free;
    end;
{$endif}

You should use TObjectDictionary that takes ownership on the values. This will work without the leaks on all platforms.

 PoiDict := TObjectDictionary<string, TMyPOI>.Create([doOwnsValues]);

Next thing, myPOI is an object, so you don't have to add it again with changed timestamp, you can just change timestamp directly - PoiDict.TryGetValue(username, myPOI) will just give you the reference it will not create copy of the object.

Corrected code would look like:

function TForm2.CreateOrUpdate(username: String; NewTimestamp: TDateTime): String;
var
  poiTimeStamp: TDateTime;
  myPOI: TMyPOI;
  Index: Integer;
begin

  if PoiDict.TryGetValue(username, myPOI) then
  begin
    // existing POI
    Result := InttoStr(myPOI.Value) + ' ' + DateTimeToStr(myPoi.Timestamp);
    poiTimestamp := myPOI.Timestamp;
    // update the Poi's timestamp - this will update object in dictionary 
    myPOI.Timestamp := NewTimeStamp;
  end
  else
  begin
    // add a new POI
    Index := Random(999);
    Result := IntToStr(Index) + ' ' + DateTimeToStr(NewTimeStamp);
    myPOI := TMyPOI.Create;
    try
      myPOI.Value := Index;
      myPOI.Timestamp := NewTimeStamp;
      PoiDict.Add(username, myPOI);
    except
      myPOI.Free;
      raise;
    end;
  end;

end;

initialization
  PoiDict := TObjectDictionary<string, TMyPOI>.Create([doOwnsValues]);

finalization
  PoiDict.Free;

end.