Is there, besides hunting for memory leaks, another situation where I should free all objects when destroying an application?

Question

Suppose an application with some forms and only one data module are created at start. In the DM1.OnCreate event, a TStringList is created to be used at runtime. We know that when the application is being terminated, all things will be destroyed and memory will automatically freed. Freeing something can take some time, and so is not always advised to worry about memory leaks on shutdown. See for example this answer from Barry Kelly or this post from Raymond Chen.

Beside that, FastMM reports the memory leak if I don't add TStringList.Free to DM1.OnDestroy. This turns out to be a problem when searching for any other memory leaks that I should really worry about.

So basically I am asking if/why/when I should free object instances that will be freed by application or OS (Windows in this specific case). Is there any other valid case that is not when hunting for memory leaks?

NOTE: In this specific case, the data module is not created or recreated more times. There will not be any memory leak at all, besides the one. The data module scrap source:

unit UDM1;

interface

uses SysUtils, Classes, ...;

type
  TDM1 = class(TDataModule)
    procedure DataModuleCreate(Sender: TObject);
    procedure DataModuleDestroy(Sender: TObject);
    procedure DoStuffWithStringList1(Sender: TObject);
  private
    internalStL: TStringList;
  end;

var
  DM1: TDM1;

implementation

procedure TDMInterfacePAFECF.DataModuleCreate(Sender: TObject);
begin
  internalStL := TStringList.Create();
end;

procedure TDMInterfacePAFECF.DataModuleDestroy(Sender: TObject);
begin
  internalStL.Free; //<-- IS THIS NECESSARY OR ADVISED?
end;

procedure DoStuffWithStringList(Sender: TObject);
begin
  //Place some code using internalStL here...

end;

Greening

you forgot "end." at the end (: kidding, I always free memory that I'm allocating, but that's just me, it's true that Windows will free it for you on application exit, but it is also true that in many cases you'll never know for sure how your application will behave in the future, so if you're creating instance(s) on datamodule's Create, it would be wise(IMHO) to free it/them on Destroy, that's my $0.02 (: – Important

Answer 1

For the same reason I strongly advocates (understatement) for not leaving any Compiler Hint or Warning in a project, clean after yourself and DO NOT LEAVE A REPORTED MEMORY LEAK!
EVER!

Now, that does not necessarily means that you have to Free everything in the Destructor of your DataModule if you have a strong case for not doing it, but in that case, you need to register your Memory Leak so that it will not be reported. (And put there a very visible comment to justify and explain why)

But consider the fact that you may leave this project and a year from now, someone else is maintaining it and has a new business requirement to create multiple DataModules... Chances are that if they do not know the inside of your code well enough, they will trust your code to be clean and problems are likely to follow.

So I would strongly advise against not freeing unless in a very special and expected and documented case...

PS: Seen that and had to mop up the memory dripping all over the place so many times that I even did some CodeRage sessions on fighting memory leaks...

Updayte: Here's the link to download that CodeRage session...

Ovid

I am accepting this. Anyway, for future reference, must note that @NGLN answer show the way to register the expected memory leak. – Greening

@EMBarbosa, for future reference, I added the link to a CodeRage session; a bit old but filled with still relevant information (registering expected memory leak included). :-) – Ovid

Answer 2

My answer can be considered philosophical, but the main reason is that any action (or absence of it) has consequences. I thought about your example and probably other examples and I see one good reason to free the object. Every time I think I can ignore freeing object increases the probability of not doing this in other, maybe more serious situation. Another example is a habit of doing "try finally end" everywhere something is allocated or freed. I don't care about freeing in case of exception, but this habit helps me avoid leaking

Output

+1 In the example above: what if in some point in future you decide to reuse your DM = create multiple instances of it. Every creation of DM creates TStringList. Every destruction of DM does not destroy TStringList = You've at least wasted memory. Which might even grow worse if you make your DM to be a part of loooooong running windows service. I personally do as much as I can to free every object I create just to avoid possible problems when the code is reused. – Thrush

Answer 3

Use RegisterExpectedMemoryLeak for the intentional memory leaks. The routine has a few overloaded versions of which one can be fead with an object class:

begin
  RegisterExpectedMemoryLeak(TStringList);
  FStringList := TStringList.Create;
  ...

or better, register the pointer itself:

begin
  FStringList := TStringList.Create;
  RegisterExpectedMemoryLeak(FStringList);
  ...

In this case the unexpected memory leaks will show up normally and can not be confused with this particular string list.

Cimabue

But why is this better than just freeing the object? It's longer to type, harder to understand, potentially slower, an unnecessary exception to the usual memory management rules, ... – Grecize

@Smasher Yes, but it's what OP wants. Like he says: Freeing something can take some time. Maybe the string list isn't a good example in that respect, but a thread surely might be. – Cimabue

@Smasher In fact, if you see the two links I've posted you will find that not losing time in freeing something that will be freed is expected and not against the memory management rules at all. – Greening

good links. I disagree anyway, maybe it's a matter of personal taste. For me it's much easier to always properly destroy what I create. But maybe I change my mind when I run into shutdown speed issues. – Grecize

@Smasher I can understand you. I do think it's a matter of personal taste when you are not affected by the slowdown. So I don't advocate this kind of implementation if you don't need it. Then, in my comments, I was just trying to show you this could be expected If not, for what would the RegisterExpectedMemoryLeak procedure be created? Thanks for your comment. – Greening

agreed, there's one more scenario for RegisterExpectedMemoryLeak though: memory leaks in code you cannot change (packages etc.) – Grecize

Answer 4

Let me answer by asking you a question.

Can you say for certain that the life cycle of the data module will always be tied to the lifetime of the application or that you will never need to create additional instances of it?

If you answer yes then feel free to ignore standard memory management practices.

If you answer no then you should make sure object's clean up after themselves.

Molality

Yes. See the note in the question. So do you do think that, beside hunting for memory leaks, there is no any other motivation to put that free? – Greening

To avoid reinforcing a bad habit perhaps. I'll say it a different way. The moment someone changes the code so your assumption about the object's lifetime no longer holds true a real memory leak has been introduced. – Molality

Beside in this example case it will not occurs, now you did a good point. +1 – Greening

Answer 5

Stuff gets done when you free an object, possibly more then only deallocating the memory.

Comes to mind a database object that does transactions and ends all started transactions in the ondestroy.

If you don't call free, ondestroy will not get fired and you may end up with locked tables.

Magdaleno