Passing objects as parameters - Cannot NIL an object

Question

Hi. I have this two functions:

procedure TDisplay.CubAssign(VAR Obj: TCubObj; CONST bReleaseOnExit: boolean);
begin
 ReleaseCubOnExit:= bReleaseOnExit;                             
 FCub:= Obj;    
 if CubReady
 then
  begin
   Init;
   SetScrollBar;
  end
 else Clear;
end;

procedure TDisplay.CubRelease;                                            
begin
 if FCub<> NIL
 then
  TRY
   FreeAndNil(FCub);
  EXCEPT
   MesajErrDetail('CubRelease', 'Cannot free object');
  END
 else FCub:= NIL;            
 Clear;   
end;

I place my TDisplay on a form and I create and assign a Cube object to TDisplay via CubAssign. Later I free that Cube by calling TDisplay.CubRelease. Now, when I close the form I don't know if my Cube was freed or not so I check it and if not NIL, I free it:

procedure TForm1.FormDestroy(Sender: TObject);
begin
 Display.CubRelease;
 if Cub<> NIL
 then FreeAndNil(Cub);
end;

However, at this point, the Cube is empty but not NIL. When FormDestroy is called, the program gives an 'Multi Free memory leak' error. Why? I already called TDiplay.CubRelease. Shouldn't it be NIL? The message I got suggests that the object was correctly freed, however it is not NIL.

Which is the correct way to implement this?

---

Edit/Clarifications

The owner of the Cube cannot be determined exactly because the parent of Display is freeing the Display while still keeping the cube(s) for a while. In other words displaying the cube in display may be a life-time operation or it may take place only for a while. Also, in some cases I may not display the cube at all.

In other words, when the main form closes, the display may be or may not be present.

In other implementations (a simple viewer where all I want to do is only to display the cube), I want to let the Display to handle the destruction of the cubes because I don't want to keep and extra list of objects to store the cubes. In this case, basically, the Displays act like storage (owners) for cubes.

Answer 1

The correct way to implement this is to always be clear about who is responsible for every object. Who owns the cube?

If your display object owns the cube, then nobody else should ever attempt to free it. According to this code, calling CubAssign transfers ownership to the display object because the display object always frees the cube object. Therefore, any code that calls CubAssign must remember to never try to free the object itself.

One way to do that is to assign nil to the original cube reference. That way, the caller can't be tempted to free the object because it won't have any reference to it anyway.

Another way is to set a Boolean value somewhere. When the code calls CubAssign, it should afterward assign False to the associated Boolean value, like this:

CubAssign(Cub, ReleaseCubOnExit);
IOwnCub := not ReleaseCubOnExit;

Then, when you're about the free the cube, check whether you own it first:

procedure TForm1.FormDestroy(Sender: TObject);
begin
  Display.CubRelease;
  if IOwnCub then
    Cub.Free;
end;

You claim that you don't know whether CubRelease actually freed anything. In fact you do, though, because the implementation you showed above always frees the object. I suspect you meant to make use of the ReleaseCubOnExit property like this:

procedure TDisplay.CubRelease;                                            
begin
  if ReleaseCubOnExit then
    FCub.Free;
  FCub := nil;
  Clear;   
end;

The exception-catching code you had was pointless since it doesn't really solve what caused the exception, so I've removed it. I've also removed the check for whether FCub is a null reference because it doesn't matter. It's always safe to call FreeOnNil on a null reference, so don't bother checking beforehand. It just clutters your code. The FreeOnNil call itself was a little pointless, too, since you need the variable to be nil regardless of whether there's anything to free.

Once your display object is honoring the ReleaseCubOnExit property, your other code can use it, too. Instead of keeping track of ownership with that IOwnCub variable I mentioned before, you could use the display's property, like this:

procedure TForm1.FormDestroy(Sender: TObject);
begin
  Display.CubRelease;
  if not Display.ReleaseCubOnExit then
    Cub.Free;
  Cub := nil;
end;

---

So, why, when you free FCub, is Cub not also set to nil? Because that's not how variables work. They are two separate variables. You know this already, in fact. They start life as two variables. One belongs to the form class, and one belongs to the display class. You initialize the form's variable, probably something like this:

Cub := TCube.Create;

Does that also set the value of the display object's FCub variable? No, of course not. Why should it? For FCub to get a value, you needed to assign it later, in the CubAssign method. They're two separate variables. Changes you make to the value of one does not affect the value of the other. Maybe the object part is confusing you. You know that separate integer variables aren't affected by each other, right?

var
  x, y: Integer;

x := 4;
y := x;
x := 3;
Assert(y = 4);

Although we assigned y using x, we can make further changes to x without affecting y. The assertion passes because y continues to retain its previous value, 4. The same goes for variables of object-reference type:

var
  x, y: TObject;

x := TObject.Create;
y := x;
x := nil;
Assert(y <> nil);

We changed the value of x, but the value of y remained intact.

Even though two variables refer to the same object, they are still two separate variables. The object itself exists independently of the two variables referring to it. Maybe a diagram will help.

Cub
+-----+
|  o----+
+-----+ |
        \    object
         +-->+-------+
FCub    /    |       |
+-----+ |    |       |
|  o----+    +-------+
+-----+

Two variables referring to a single object.

Calling Free on a variable does not change the value of the variable. It only destroys the object referred to by the variable. That's why there are two different functions, Free and FreeAndNil. The latter assigns nil to the variable passed in. As we've established above, assigning a value to one variable won't change any others that happen to have the same value, so after you call FreeAndNil(FCub), the diagram above changes to look like this:

Cub
+-----+
|  o------> ???
+-----+


FCub
+-----+
| nil |
+-----+

Cub is what we call a dangling reference because the arrow is just dangling off in space, not pointing to anything valid anymore.

---

So, how do you fix this? The display object doesn't know about the other reference to the cube object. You could give the display a reference to the form at the same time you give it a reference to the cube:

procedure TDisplay.CubAssign(Obj: TCube; Form: TForm1; ReleaseOnExit);
begin
  FCub := Obj;
  FForm := Form;
  FReleaseOnExit := ReleaseOnExit;
end;

Then, when the cube is freed, clear the reference on the form, too:

FreeAndNil(FCub);
FForm.Cub := nil;

That creates what's called tight coupling; the two classes now cannot exist apart from each other because the display form requires a TForm1 instance. It won't work with any other kinds of forms, and the cubes it holds must belong to the form.

Tight coupling is generally a bad idea. It will fix this specific problem, so it probably looks good to you right now, but it will eventually stifle development of your program because you won't be able to reuse anything. A better solution to your dangling-reference problem is given by the first sentence of this answer. If the cube object can be destroyed at any time without the form's knowledge, then the form shouldn't be using its Cub variable anymore because it doesn't own the object it refers to.

You could mitigate the problem by giving the cube a list, where it can keep track of anyone who's interested in knowing about its destruction. The list could be one of TNotifyEvent method pointers. As the cube object is being destroyed, it can go through the list and call each of the method pointers. The form object will have previously registered a method with the cube object. When that method gets called, the form can clear its own reference to the cube. This way, the cube doesn't need to know where all its references are, and neither does the display. The form can use the Cub variable as long as the destruction method doesn't get called. This technique of notifications to interested parties is known as the observer pattern.