ARC: is there a memory leak in TNetEncoding.GetBase64Encoding?

Question

Below is the original Delphi source code of TNetEncoding.GetBase64Encoding. But I suspect there will be a memory leak in case AtomicCmpExchange(Pointer(FBase64Encoding), Pointer(LEncoding), nil) <> nil

 TNetEncoding = class
  private
    class var
      FBase64Encoding: TNetEncoding;

with

class function TNetEncoding.GetBase64Encoding: TNetEncoding;
var
  LEncoding: TBase64Encoding;
begin
  if FBase64Encoding = nil then
  begin
    LEncoding := TBase64Encoding.Create;
    if AtomicCmpExchange(Pointer(FBase64Encoding), Pointer(LEncoding), nil) <> nil then
      LEncoding.Free;
{$IFDEF AUTOREFCOUNT}
    FBase64Encoding.__ObjAddRef;
{$ENDIF AUTOREFCOUNT}
  end;
  Result := FBase64Encoding;
end;

I think it must be written as:

class function TNetEncoding.GetBase64Encoding: TNetEncoding;
var
  LEncoding: TBase64Encoding;
begin
  if FBase64Encoding = nil then
  begin
    LEncoding := TBase64Encoding.Create;
    if AtomicCmpExchange(Pointer(FBase64Encoding), Pointer(LEncoding), nil) <> nil then
      LEncoding.Free
{$IFDEF AUTOREFCOUNT}
    !!!ELSE!!! FBase64Encoding.__ObjAddRef;
{$ENDIF AUTOREFCOUNT}
  end;
  Result := FBase64Encoding;
end;

Am I wrong?

Answer 1

You are correct. The code fails when the if statement evaluates true. When that happens another thread has managed to assign the singleton first, and already increased the ref count. The ref count should not be increased again.

You should submit a bug report.