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;

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 (: — , Mar 27 '12 at 19:23

score 7 · Answer 1 · answered Mar 27 '12 at 19:05

7

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

answered Mar 27 '12 at 19:05

Maksee

2,231
2
22
31

1

+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. – Tom Hagen Mar 28 '12 at 06:42

Francesca · Accepted Answer · 2012-04-10T17:11:57.333

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...

I am accepting this. Anyway, for future reference, must note that @NGLN answer show the way to register the expected memory leak. — EMBarbosa, Apr 10 '12 at 03:00
@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). :-) — Francesca, Apr 10 '12 at 17:15

NGLN · Answer 3 · 2012-03-27T19:37:07.240

5

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.

edited Mar 27 '12 at 19:37

answered Mar 27 '12 at 19:22

NGLN

41,230
8
102
186

1

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, ... – jpfollenius Mar 27 '12 at 20:20
1

@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](http://stackoverflow.com/q/9029730/757830). – NGLN Mar 27 '12 at 20:25
@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. – EMBarbosa Mar 28 '12 at 01:47
3

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. – jpfollenius Mar 28 '12 at 07:31
@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. – EMBarbosa Mar 28 '12 at 13:34
1

agreed, there's one more scenario for `RegisterExpectedMemoryLeak` though: memory leaks in code you cannot change (packages etc.) – jpfollenius Mar 28 '12 at 14:13

Kenneth Cochran · Answer 4 · 2012-03-27T19:13:07.330

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.

edited Mar 27 '12 at 19:13

answered Mar 27 '12 at 18:59

Kenneth Cochran

11,576
3
46
110

1

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? – EMBarbosa Mar 27 '12 at 19:04
6

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. – Kenneth Cochran Mar 27 '12 at 19:18
Beside in this example case it will not occurs, now you did a good point. +1 – EMBarbosa Mar 27 '12 at 19:41

score 2 · Answer 5 · answered Mar 28 '12 at 08:44

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.

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

5 Answers5