views:

529

answers:

3

I inherited an Intraweb app that had a 2MB text file of memory leaks as reported by FastMM4. I've got it down to 115 instances of one class leaking 52 bytes.

A brief description of the bad actor is:

TCwcBasicAdapter = class(TCwcCustomAdapter)  
  protected  
    FNavTitleField: TField;  
    function GetAdapterNav(aDataSet: TDataSet): ICwcCDSAdapterNav; override;  
  public  
    constructor Create(aDataSource: TDataSource; aKeyField, aNavTitleField: TField; aMultiple: boolean);  
  end;

and the interface is:

  ICwcCDSAdapterNav = interface(IInterface)

Am I barking up the wrong tree, since the property is reference counted? Are there any circumstances where the interface property could keep the class from being destroyed?

Here is the implementation of the method above:

function TCwcBasicAdapter.GetAdapterNav(aDataSet: TDataSet): ICwcCDSAdapterNav;
var
  AdapterNav: TCwcCDSAdapterNavBase;
begin
  result := nil;
  if Assigned(aDataSet) then begin
    AdapterNav := TCwcCDSAdapterNavBasic.Create(aDataSet, FKeyField.Index, FNavTitleField.Index);
    try
      AdapterNav.GetInterface(ICwcCDSAdapterNav, result);
    except
      FreeAndNil(AdapterNav);
      raise;
    end;
  end;
end;

with the class declared as:

TCwcCDSAdapterNavBase = class(TInterfacedObject, ICwcCDSAdapterNav)
+4  A: 

FastMM should give you what is leaked and where it was created.
That would help narrowing it down to the real culprit: who is leaking what?

I'm not sure what really your question is?
Your code is incomplete or not the one in question: your class does not have an Interface property nor an Interface private Field, just a method that returns an Interface, which is harmless.

Edit: Without seeing the code of your Object implementing ICwcCDSAdapterNav, we can't tell if it is indeed reference counted.
If you don't descend from TInterfacedObject, chances are that it's not reference counted and that you cannot rely on this automagically freeing...

You may want to give a look at this CodeRage 2 session: Fighting Memory Leaks for Dummies. It mainly shows how to use FastMM to prevent/detect memory leaks in Delphi. Was for D2007 but still relevant for other versions.

François
Thanks, I'll look your presentation over at work this morning. You got cut short at DelphiLive!.
Do you have a presentation that describes some of the techniques listed on the "Passing Around Leaky Containers" slide? Also, I downloaded your DelphiLive! presentation, but don't see a way to view it. Is it missing files?
I've selected this answer because the referenced presentation helped me find the real problem, albeit not solved yet.
+2  A: 

If you are leaking 115 instances of that class, then it is that class that is being leaked. The memory occupied by that class, not the memory occupied by the things it refers to, is being leaked. Somewhere, you have 115 instances of TCwcBasicAdapter that you're not freeing.

Furthermore, properties don't store data, no matter they're interfaces or some other type. Only fields occupy memory (along with some hidden space the compiler allocates on the class's behalf).

So, yes, you are barking up the wrong tree. Your memory leak is somewhere else. When FastMM tells you that you have a memory leak, doesn't it also tell you where each leaked instance was allocated. It has that capability; you might need to adjust some conditional-compilation symbols to enable that feature.

Surely it's not only instances of that class that are leaking, though. FastMM should also report some other things leaking, such as instances of the class or classes that implement the interface.


Based on the function you added, I've begun to suspect that it's really TCwcCDSAdapterNavBase that's leaking, and that could be because of the atypical way you use for creating it. Does the exception handler in GetAdapterNav ever run? I doubt it; TObject.GetInterface never explicitly raises an exception. If the object doesn't support the interface, it returns False. All that exception handler could catch are things like access violation and illegal operations, which you really shouldn't be catching there anyway.

You can implement that function more directly like this:

if Assigned(FDataSet) then
  Result := TCwcCDSAdapterNavBase.Create(...);
Rob Kennedy
Looks like he's already getting the log file about what's being leaked. But the relation between knowing where it gets constructed and knowing when to destroy it is not always a simple or obvious one.
Mason Wheeler
If you know who created the leaked objects, then that often tells you who's supposed to be responsible for freeing them. And I know he already has the log about leaks. But I'm suggesting that maybe he hasn't looked at the whole log since it's unlikely that just instances of that one class have leaked. The things the class allocated for itself probably also leaked, as well as the objects those interface references point to. And finally, the main point is that the interface property cannot be the cause of the leaked instances of that class.
Rob Kennedy
The FastMM4 report references the leaked class' creation, where it is added to a TObjectList member of a TObject descendant. The TObjectList is created as not Owning, and is FreeAndNil in the class' destructor after a for loop calling Remove on its Items. The frustrating part is all that is happening as expected.
So, if the TObjectList doesn't own those created objects, then who does? What code was intended to be responsible for cleaning up those objects? Do you have ANY code that frees them? If not, then add some (which might be as simple as making the list own the objects). If so, then find out why that code isn't being executed.
Rob Kennedy
That could be a very bad idea, depending. If he's relying on the reference counting system to free them, and some of them are getting disposed of properly so the 115 that are leaking aren't all of the instances in that list, then setting the list to own the objects will lead to double-free errors.
Mason Wheeler
What Mason said. If I change the TObjectList Owning to True, it causes AVs in the destructor.
I'll start checking the function GetAdapterNav tomorrow. Had to deploy another app today.
So, what objects are you adding to the TObjectList? Certainly not the ICwcCDSAdapterNav interface references that GetAdapterNav is returning, I hope. Those aren't objects. Those are interfaces. There's an object underneath, but you lose that the moment you start referring to it via an interface. If you want a list of interfaces, use a TInterfaceList, not a TObjectList. If you've been keeping interfaces in a TObjectList, then you've been type-casting to get them back out. That can certainly lead to reference counts remaining higher than they should be.
Rob Kennedy
+4  A: 

You've got some good answers so far about how FastMM works. But as for your actual question, yes, interfaced objects can leak in two different ways.

  1. Interfaces are only reference-counted if the objects they belong to have implemented reference counting in their _AddRef and _Release methods. Some objects don't.
  2. If you have circular interface references, (Interface 1 references interface 2, which references interface 1,) then the reference count will never fall to 0 without some special tricks on your part. If this is your problem, I'll refer you to Andreas Hausladen's recent blog post on the subject.
Mason Wheeler
Thanks, I suspect item 2 above is occuring.
You could also check for odd usage such as casts.For tracking down leaks FastMM certainly helps but I would suggest you give AQTime http://www.automatedqa.com/products/aqtime/ a try. I know it isn't free but knowing the full stack trace for each leak certainly improves the time it takes to clean up the code.
Ryan VanIderstine