Shouldn't calling Free on an object reference set to nil throw an Access Violation every time it is called?

Question

I'm getting access violations from the unit DBXCommon.pas (in Delphi XE). When I look at the code I see things like the following (at the exclamation marks):

function TDBXConnectionFactory.GetConnection(const DBXContext: TDBXContext;
  const ConnectionProperties: TDBXProperties): TDBXConnection;
var
  ConnectionBuilder:  TDBXConnectionBuilder;
  DelegatePath:       TDBXDelegateItem;
  Connection:         TDBXConnection;
  CombinedProperties: TDBXProperties;
begin
  //...
  ConnectionBuilder := TDBXConnectionBuilder.Create;
  Connection        := nil;
  try
    //..lots of setting ConnectionBuilder properties
    ConnectionBuilder.FInputPassword := CombinedProperties[TDBXPropertyNames.Password];
    Connection := ConnectionBuilder.CreateConnection;
    Connection.Open;
    Result     := Connection;
!!  Connection := nil;
  finally
!!  Connection.Free;
    ConnectionBuilder.Free;
  end;
end;

But I see constructs like this (first assign Nil, then a Free) much more in DBXCommon.pas. Is this some construct I do not know, or is this really causing access violation every time this piece of code is called?

Answer 1

Calling Free on a null reference is always safe. Go look in the implementation of TObject.Free to see why.

This code is an example of a factory function. Its job is to create a new instance of a class, but if it fails, it needs to make sure it doesn't leak a half-created instance when it throws an exception, so it calls Free. When it's sure it's going to succeed, it transfers ownership of the result to the caller. It still calls Free, but if it's already transfered ownership, then it ends up calling Free on a null reference, and there's no harm done. This code is what transfers ownership:

Result := Connection;
Connection := nil;

The way I would write a factory function would do away with the separate Connection variable. I'd construct the result directly in Result, but free it if there were an exception, like this:

function TDBXConnectionFactory.GetConnection(const DBXContext: TDBXContext;
  const ConnectionProperties: TDBXProperties): TDBXConnection;
var
  ConnectionBuilder:  TDBXConnectionBuilder;
  DelegatePath:       TDBXDelegateItem;
  Connection:         TDBXConnection;
  CombinedProperties: TDBXProperties;
begin
  //...
  ConnectionBuilder := TDBXConnectionBuilder.Create;
  try
    //..lots of setting ConnectionBuilder properties
    ConnectionBuilder.FInputPassword := CombinedProperties[TDBXPropertyNames.Password];
    Result := ConnectionBuilder.CreateConnection;
    try
      Result.Open;
    except
      Result.Free;
      raise;
    end;
  finally
    ConnectionBuilder.Free;
  end;
end;

That has the same effect.