Potential memory leaks by initializing a record

Delphi uses reference-counting with copy-on-write semantics [1][2] to reduce memory allocation for strings (not for WideString). A kind of memory leak was found by accident. Let us first look the following example:

type
  TFoo = record
    StringField: string;
  end;

procedure CreateLeakTest;
var
  Foo: TFoo;
begin
  FillChar(Foo, SizeOf(Foo), 0);
  Foo.StringField := 'Leak Test';
  FillChar(Foo, SizeOf(Foo), 0); //<--- A leak!
end;

Initializing records with FillChar() is quite common in Delphi. By calling FillChar() twice, you might create a memory leak. Note that no leaks on ShortString. My assumption: FillChar() is unsafe to cleanup records with ref-counted fields.

function StringStatus(const S: string): string;
begin
  Result := Format('Addr: %p, Refc: %d, Val: %s',
    [Pointer(S), PInteger(Integer(S) - 8)^, S]);
end;

procedure Diagnose;
var
  S: string;
  Foo: TFoo;
begin
  S := Copy('Leak Test', 1, 5); // Force to allocate a new string
  WriteLn(StringStatus(S));
  Foo.StringField := S;
  WriteLn(StringStatus(Foo.StringField));
  FillChar(Foo, SizeOf(Foo), 0);
  WriteLn(StringStatus(S));
end;

The output of the above code looks as follows:

Addr: 00E249E8, Refc: 1, Val: Leak Test // A string buffer is allocated
Addr: 00E249E8, Refc: 2, Val: Leak Test // Its Refc is incremented
Addr: 00E249E8, Refc: 2, Val: Leak Test // Its Refc should equal 1 (unexpected)

After calling FillChar(), the StringField is pointed to nil. However the reference count of its previous string buffer hasn't been decremented, so that its reference count will NEVER go back to 0. In other words, this string buffer will not be deallocated before your program is terminated. This is a leak.

How to initialize a record in a safe way?

As ref-counted fields are not handled correctly by using FillChar(). The default way of initializing a record looks more like an abuse of FillChar. I suggest declare a const record with initial values instead of using FillChar.

const
  EmptyRecordX: TRecordX = (
    Field1: InitVal1;
    Field2: InitVal2;
    ...
    FidldN: InitValN
  );

// In your application
var
  Foo: TRecordX;
begin
  Foo := EmptyRecordX; // instead of FillChar(Foo, SizeOf(Foo), 0);
  //...

It is quite safe to initialize a record in this way, isn't it?

Alternative Solution

If you are too lazy to declare such empty record constants. The following function can help you as well. Note that it is a little bit tricky.

procedure InitRecord(out R; RecordSize: Integer);
begin
  FillChar(R, RecordSize, 0);
end;

Thanks for the magic word "out". As it is in Help described "An out parameter, like a variable parameter, is passed by reference. With an out parameter, however, the initial value of the referenced variable is discarded by the routine it is passed to. The out parameter is for output only; that is, it tells the function or procedure where to store output, but doesn't provide any input. "

Let us see what the code actually has done to a record.

mov edx,[$0040c904]
mov eax,ebx
call @FinalizeRecord  //<----- cleanup
mov edx,$0000000c
call InitializeRecord

Compile calls procedure FinalizeRecord(), so that a record will be completely finalized.

UPDATE #1: As Jonas Maebe recently described: "If you have local record which was declared but not yet used, a simple fillchar(rec,sizeof(rec),0) will set everything to 0/nil/empty. If it may have been used earlier and contains ref-counted fields, you first have to call finalize(rec). "[3] His argument is more understandable and closer to the point of the issue.

References

1. Wikipedia: Copy-on-write
2. A Brief History of Strings
3. fpc-pascal maillist