memory leak vs. access violation, class design issue

Question

I show a short design pattern for a user class in the code below.

type
  MytestClass = class
    alist: TStringlist;
  public
    constructor Create;
    destructor destroy; override;
  end;
  { MytestClass }

type
  TForm1 = class(TForm)
    btn_version01: TBitBtn;
    btnversion02: TBitBtn;
    procedure FormClose(Sender: TObject; var Action: TCloseAction);
    procedure FormCreate(Sender: TObject);
    procedure btn_version01Click(Sender: TObject);
    procedure btnversion02Click(Sender: TObject);
  private
    { Private-Deklarationen }
  public
    { Public-Deklarationen }

    btestClass : MytestClass;
    aComplexClassDesign : TComplexClassDesign;
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}

constructor MytestClass.Create;
begin
  alist := TStringlist.Create;
end;

destructor MytestClass.destroy;
begin
  alist.free;
  inherited;
end;


procedure TForm1.btnversion02Click(Sender: TObject);
var atestClass : MytestClass;
begin
     ///
  atestClass :=MytestClass.Create;
  atestClass.Free;
  atestClass := nil;
end;

procedure TForm1.btn_version01Click(Sender: TObject);
var atestClass : MytestClass;
begin
     ///
  atestClass :=MytestClass.Create;
  atestClass.Free;
end;

procedure TForm1.FormClose(Sender: TObject; var Action: TCloseAction);
begin
  btestClass.free;
  aComplexClassDesign.Free;
end;

procedure TForm1.FormCreate(Sender: TObject);
begin
  btestClass :=MytestClass.Create;
  aComplexClassDesign :=TComplexClassDesign.Create;
end;

end.

Hope this is a perfect design without Memory leaks and Access violations. All the classes I use in my real application are designed along this pattern.

FastMM4 does not show up any problem on my TComplexClassDesign in the code above. In the real application FASTMM4 is reporting a Memory leak for my TComplexClassDesign, even I call the free function in the Close Event of the form. If I step the through the code for sure this function is executed. Any idea how to debug this memory leak report, any option to see the instance of TComplexClassDesign which has not been released? Any other reason why I got this strange memory leak report ?

Bonus question :

DUnit always make teardown code like this

  atestClass :=MytestClass.Create;
  atestClass.Free;
  atestClass := Nil

Is the last line of code really needed ?

`>Is the last line of code really needed ?`. No it is not needed. If the variable `atestClass` were to be reused, it would be a good idea to nil it, but otherwise no. It has been up to many discussions over the years though, since in some cases it can make debugging easier. Search for `FreeAndNil()`. [Which is preferable: Free or FreeAndNil?](http://stackoverflow.com/questions/3159376/which-is-preferable-free-or-freeandnil). — LU RD, May 29 '15 at 06:54

David Heffernan · Answer 1 · 2015-05-29T09:02:29.353

It is a mistake to match OnCreate with OnClose. Those events are not a pair. The OnCreate event is called during construction. The matching event for destruction is OnDestroy. Anything that you create in OnCreate should be destroyed in OnDestroy.

Personally I see little value in OnCreate and OnDestroy. I would always opt to override the virtual constructor and destructor.

procedure TForm1.FormCreate(Sender: TObject);
begin
  btestClass :=MytestClass.Create;
  aComplexClassDesign :=TComplexClassDesign.Create;
end;

procedure TForm1.FormClose(Sender: TObject; var Action: TCloseAction);
begin
  btestClass.free;
  aComplexClassDesign.Free;
end;

As a general rule, destroy objects in reverse order of creation. It doesn't matter here, but if the objects have dependencies then the order is sometimes important.

procedure TForm1.btnversion02Click(Sender: TObject);
var atestClass : MytestClass;
begin
  atestClass :=MytestClass.Create;
  atestClass.Free;
  atestClass := nil;
end;

Is there any point to assigning nil to atestClass? Absolutely not. This variable leaves scope immediately following the assignment and so the result assignment cannot be observed by any other code. I would expect the compiler to warn about this. It warns when you make assignments but then never use the assigned value. I hope you have enabled warnings.

Finally, why does your real code have a memory leak? Impossible to tell from here. The full debug version of FastMM gives leak reports with informative stack traces. Those traces will include the call stack that led to the allocation of the leaked objects.

lol - Why bother writing up a detailed answer to the *actual* question when all you needed to do was to answer the *bonus* question. — Lieven Keersmaekers, May 29 '15 at 08:57
yeah, I have no idea why my answer was marked as the accepted answer. clearly I didn't answer the main question and I made no claims that I was. I clearly stated that I was attempting to answer the bonus question. as for downvotes or upvotes, I was not involved in any part of that. it is strange to get downvoted because the community didn't like my answer being accepted. yeah, it wasn't the right answer for the main question, but that could have been handled by fixing the accepted answer. — mbrandeis, Jun 01 '15 at 22:23

score -1 · Accepted Answer · answered May 29 '15 at 06:52

-1

Bonus answer:

atestClass will still be pointing to a memory address even though you called free. if you tried to access the memory after calling free you could be accessing the wrong data if something else is now using that memory location, or get an access violation if the memory is not accessible anymore.

if free is the last line in a function and then it exits/returns then it may not be needed...

answered May 29 '15 at 06:52

mbrandeis

250
1
2

imo - the accepted answer should switch to the one David gave but downvoting or voting to delete this answer is not the correct either. – Lieven Keersmaekers May 30 '15 at 13:33
@Lieven I find it interesting that downvoting is often criticised but upvoting seldom criticised. One wonders why the asymmetry. – David Heffernan May 30 '15 at 20:25
I assume it's all about [positive and negative feedback](https://www.psychologytoday.com/blog/happiness-in-world/201309/which-kind-feedback-is-best) but in this case: this answer is "not wrong". The problem is the question and what became the accepted answer but that's not @mbrandeis doing so to me, there's no point in downvoting. – Lieven Keersmaekers May 30 '15 at 22:14

memory leak vs. access violation, class design issue

2 Answers2