Freeing Multiply-referenced Objects

Question

This is another post about me inheriting an Intraweb app that had a 2MB text file of memory leaks as reported by FastMM4, where I've got it down to 115 instances of one class leaking 52 bytes each.

The leaks are from a rather convoluted instantiation and handling of the class. Each instantiation of the class is needed to get the app to work right now. So I'm looking for some ways to either clone the class with some straight-forward cleanup of the clone, or referencing in a different way, or..?

The first instantiation of the class (TCwcBasicAdapter) is as a local variable that gets added to a TObjectList (not Owning) and destroyed with the TObjectList (FCDSAdapters):

procedure TCwcDeclaration.AttachAdapter(DS: TDataSource; const FormName, KeyFN, TitleFN: string; const Multiple: boolean = False;
  const AllowAttachment: boolean = False; const AllowComment: boolean = False);  
var  
  Forms : TCwcSessionForms;  
  Adapter: TCwcCDSAdapter;  
  KeyField, TitleField: TField;  
begin  
  Forms := GetForms(FormName);  
  KeyField := DS.DataSet.FindField(KeyFN);  
  TitleField := DS.DataSet.FindField(TitleFN);  
  Adapter := TCwcBasicAdapter.Create(DS, KeyField, TitleField, Multiple);  
  Adapter.AttachDBPersist(Self.DBPersist);  
  Forms.AttachDataAdapter(Adapter);  
  Forms.SetAllowAttachments(AllowAttachment);  
  Forms.SetAllowComments(AllowComment);  
end;  

procedure TCwcSessionForms.AttachDataAdapter(aCDSAdapter: TCwcCDSAdapter);  
var  
  Index: integer;  
begin  
  if (FCDSAdapters.IndexOf(aCDSAdapter)  -1)  
    then raise Exception.CreateFmt('Duplicate Adapter attempting to be attached on %0:s', [FFormClassName]);  
  Index := FCDSAdapters.Add(aCDSAdapter);  
  if (FDefaultAdapterIndex = -1)  
    then FDefaultAdapterIndex := Index;  
end;  

The second instantiation of the class is also as a local variable that gets added to a TObjectList (not Owning) and destroyed with the TObjectList (FAdapters):

procedure TCwcCDSMulticastList.InitializeAdapters(const aSessionForms: TCwcSessionForms);  
var  
  i, Count: integer;  
  Adapter:  TCwcCDSAdapter;  
  TempMulticast: TCwcCDSEventMulticast;  
begin  
  Count := aSessionForms.GetDataAdapterCount;  
  for i := 0 to Pred(Count) do begin  
      Adapter := aSessionForms.GetDataAdapter(i);  
      TempMulticast := FindDataSource(Adapter.DataSource);  
      if (TempMulticast = nil) then begin  
          TempMulticast := TCwcCDSEventMulticast.Create(Adapter.DataSource);  
          try  
            FMulticastList.Add(TempMulticast);  
          except  
            FreeAndNil(TempMulticast);  
            raise;  
          end;  
        end;  
      TempMulticast.AddObserver(Adapter);  
      FAdapters.Add(Adapter);  
    end;  
end;  

The third instantiation of the class is as part of an observer pattern from the TempMulticast.AddObserver(Adapter) line above. The observer is added to TObjectList FObservers (Owning):

procedure TCwcCDSEventMulticast.AddObserver(const aCDSAdapter: TCwcCDSAdapter);  
begin  
  FObservers.Add(TCwcCDSAdapterObserver.Create(aCDSAdapter));  
end;  

constructor TCwcCDSAdapterObserver.Create(const aCDSAdapter: TCwcCDSAdapter);  
begin  
  inherited Create;  
  FOnStateChange     := aCDSAdapter.OnStateChangeIntercept;  
  FOnAfterDelete     := aCDSAdapter.AfterDeleteIntercept;  
  FInvalidateCursors := aCDSAdapter.InvalidateCursors;  
end;  

The TCwcBasicAdapter is leaked here, not cleaned up when FObservers is destroyed.

The latest thing I've tried is changing FObservers to not Owning, creating a private field for the Adapter, freeing the private field in TCwcCDSAdapterObserver.Destroy, but that causes errors.

Thanks,

Paul Rice

Answer 1

You realize you can dispose of objects by yourself without making their owners auto-dispose of them? I ask this because it feels like you're trying to make the automatics do the job in all cases.

Answer 2

If the lists aren't owners, then they will not free the objects when the list is freed. Just calling Remove on each item won't do it either. You would have to iterate through the list and call Free on each item in the list, and then free the list itself.

If you make the lists owners, then they will do this for you when you free the list.

for i := 0 to FAdapters.Count do Free(FAdapters[i]);
FreeAndNil(FAdapters);