这里有内存泄漏吗？

Question

这段代码是否可以防止内存泄漏？

s := TStringList.Create; // create  first object
try
  // Here line comes that seems to be dangerous
  s := GetSomeSettings; // Overrides reference to first object by second one
finally
  s.free; // Destroying only second object, leave first object to live somewhere in memory
end;


function GetSomeSettings : TStringList;
var
  rawString : string;
  settings : TStringList;
begin
  // Singleton pattern implementation

  // Trying to find already existing settings in class variable
  settings := TSettingsClass.fSettings;

  // If there is no already defined settings then get them
  if not Assigned(settings) then
  begin         
    GetSettingsInDB(rawString);
    TSettingsClass.fSettings := ParseSettingsString(rawString);
    settings := TSettingsClass.fSettings;       
  end;
  Result := settings;
end;

我想知道s := GetSomeSettings;可能有害并忽略第一个对象,将其保存在记忆中？

Answer 1

是的,第1行创建的StringList被泄露.

实际上,你在做:

s := TStringList.Create;
s := AnotherStringList;
AnotherStringList.Free;

至于GetSomeSettings常规:

通常,将新创建的实例作为函数结果返回是不明智或不鼓励的,因为您将所有权和销毁的责任转移到调用代码.除非你有一个机制/框架来处理它,这似乎是你的情况TSettingsClass,但在这一小段代码中没有足够的证据.

然而,两段代码的组合显示出另一个问题:之后s.Free,TSettingsClass.fSettings被破坏但不是零.因此,第二次GetSomeSettings被调用,它返回一个悬空指针.

Answer 2

1)你不应该问什么时候可以在两分钟内办理登机手续!

program {$AppType Console};
uses Classes, SysUtils;

type TCheckedSL = class(TStringList)
     public
       procedure BeforeDestruction; override;
       procedure AfterConstruction; override;
end;

procedure TCheckedSL.BeforeDestruction; 
begin
  inherited;
  WriteLn('List ',IntToHex(Self,8), ' going to be safely destroyed.');
end;

procedure TCheckedSL.AfterConstruction; 
begin
  WriteLn('List ',IntToHex(Self,8), ' was created - check whether it is has matched  destruction.');
  inherited;
end;

procedure DoTest; var s: TStrings;
 function GetSomeSettings: TStrings;
 begin Result := TCheckedSL.Create end;
begin
  Writeln('Entered DoTest procedure');
  s := TCheckedSL.Create; // create  first object
  try
    // Here line comes that seems to be dangerous
    s := GetSomeSettings; // Overrides reference to first object by second one
  finally
    s.free; // Destroying only second object, leave first object  
  end;
  Writeln('Leaving DoTest procedure');
end;

BEGIN 
  DoTest;
  Writeln;
  Writeln('Check output and press Enter when done');
  ReadLn;
END.

2)在少数利基案件中,这仍然是安全的.

1. 在FPC(http://FreePascal.org)中S可能是某个单元的"全局属性",有一个可以释放旧列表的setter.
2. 在Delphi Classic中S可以是某种接口类型,由创建的对象支持.当然,标准TStringList缺少任何接口,但是一些库(例如http://jcl.sf.net)确实提供基于接口的字符串列表,具有更丰富的API(iJclStringList类型和相关).
3. 在Delphi/LLVM中,所有对象都被引用计数,就像没有GUID的接口一样.所以代码在那里是安全的.
4. 您可以声明S为一个记录 - 所谓的Extended Record已经重新定义,class operator Implicit以便s{record} := TStringList.Create在分配新的实例之前,类型转换将释放前一个实例.这很危险,因为它非常脆弱且容易被滥用,并且在其他地方销毁列表,在S记录中留下悬空指针.
5. 你的对象可能不是那个vanilla TStringList,而是一些子类,覆盖构造函数或者AfterConstruction在某个列表中注册自己,这在某些地方是一次性的.Mark/Sweep围绕大量工作负载的堆管理.VCL TComponent可能被视为遵循以下模式:form拥有其组件并在需要时释放它们.这就是你 - 以简化形式 - 尝试用TSettingsClass.fSettingscontainter(任何参考是1大小的容器).但是这些框架确实需要一个环回:当对象被释放时,它也应该从所有容器中移除它自己,引用它.

.

procedure TCheckedSL.BeforeDestruction; 
begin
  if Self = TSettingsClass.fSettings then TSettingsClass.fSettings := nil;
  inherited;
end;

class procedure TSettingsClass.SetFSettings(Value);
var fSet2: TObject;
begin
  if fSettings <> nil then begin
     fSet2 := fSettings; 
     f_fSettings := nil; // breaking the loop-chain
     fSet2.Destroy; 
  end;
  f_fSettings := Value;
end;

class destructor TSettingsClass.Destroy;
begin
  fSettings := nil;
end;

然而,由于显然需要保持设计对称 - 注册也应该作为课程的一部分来完成.负责注销的人通常也是负责注册的人,除非有理由扭曲设计.

procedure TCheckedSL.AfterConstruction; 
begin
  inherited;
  TSettingsClass.fSettings := Self;
end;

...
if not Assigned(settings) then
  begin         
    GetSettingsInDB(rawString);
    TCheckedSL.Create.Text := ParseSettingsString(rawString);
    settings := TSettingsClass.fSettings;       
    Assert( Assigned(settings), 'wrong class used for DB settings' );
  end;
  Result := settings;