It's always wrong to use methods or properties of a null reference, even if it appears to work sometimes.
FreeAndNil indeed cannot be used to detect double frees. It is safe to call FreeAndNil on an already-nil variable. Since it's safe, it doesn't help you detect anything.
This is not a stale-pointer bug. This is a null-reference bug. A stale-pointer bug is when you have freed an object but not cleared all variables that referenced it. Then the variable still holds the old address of the object. Those are very hard to detect. You can get such a bug like this:
MStr := TMemoryStream.Create;
MStr.Free;
MStr.Size := 0;
You can also get one like this:
MStr := TMemoryStream.Create;
OtherStr := MStr;
FreeAndNil(MStr);
OtherStr.Size := 0;
Using MStr.Size after you have freed the object MStr referenced is an error, and it should raise an exception. Whether it does raise an exception depends on the implementation. Maybe it will, and maybe it won't. It's not random, though.
If you're searching for a double-free bug, you can use the debugging aides that FastMM provides, as others have suggested as well. It works by not actually releasing the memory back to the operating system, or even back to Delphi's internal free-memory pool. Instead, it writes known-bad data into the object's memory space, so when you see those values, you'll know you're reading from something that you already freed. It also modifies the object's VMT so that the next time you call a virtual method on that object reference, you'll get a predictable exception, and it will even tell you which supposedly freed object you tried to use. When you attempt to free the object again, it can tell you not only that you already freed it, but also where it was freed the first time (with a stack trace), and where it was allocated. It also collects that information to report about memory leaks, where you freed an object less than one time instead of more.
There are also habits you can use to avoid the issue for future code:
- Reduce the use of global variables. A global variable could be modified by any code throughout the program, forcing you to wonder whenever you use it, "Is this variable's value still valid, or did some other code free it already?" When you limit the scope of a variable, you reduce the amount of code you have to consider in your program when looking for reasons a variable doesn't have the value you expect.
- Be clear about who owns an object. When there are two pieces of code that have access to the same object, you need to know which of those pieces of code owns the object. They might each have a different variable for referencing the object, but there's still just one object there. If one piece of code calls
FreeAndNil on its variable, that still leave's the other code's variable unchanged. If that other code thinks it owns the object, then you're in trouble. (This concept of owner is not necessarily tied to the TComponent.Owner property. There doesn't need to be an object that owns it; it could be a general subsystem of your program.)
- Don't keep persistent references to objects you don't own. If you don't keep long-lived references to an object, then you don't have to worry about whether those references are still valid. The only persistent reference should be in the code that owns the object. Any other code that needs to use that object should receive a reference as an input parameter, use the object, and then discard the reference when it returns its result.
