How to leak a class you never defined
Quick, what’s wrong with this code?
[code lang="delphi"]
procedure ContrivedExample;
var
factorial: TFunc;
begin
factorial :=
function(input: Integer): integer
begin
if (input = 0) or (input = 1) then
result := input
else result := input * (factorial(input - 1));
end;
writeln(factorial(5));
end;
[/code]
Or how about this one, which doesn’t use recursion, and is based on something I noticed when using the list comparison algorithm to solve a problem at work?
[code lang="delphi"]
procedure SomewhatLessContrivedExample(list1, list2: TList);
var
newItem, editItem, deleteItem: TProc;
begin
newItem :=
procedure(value: TMyObject)
begin
fMainList.Add(value);
end;
editItem :=
procedure(value: TMyObject)
begin
if value.CheckSomeFlag = true then
newItem(value)
else fMainList[value.id].Update(value);
end;
deleteItem :=
procedure(value: TMyObject)
begin
fMainList.Delete(value);
end;
listCompare(list1, list2, editItem, newItem, deleteItem);
end;[/code]
If you were paying close attention in my first two posts about how anonymous methods are implemented, you might realize that these anonymous method references are interface references to a TInterfacedObject descendant, and that by having the closure capture one, the object now holds a reference to itself. Which means that when you actually run the code, you could end up with FastMM4 reporting something like this:
An unexpected memory leak has occurred. The unexpected small block leaks are:
21 – 28 bytes: ContrivedExample$ActRec x 1
Oops! That’s not good, especially if you call ContrivedExample in a main loop somewhere and end up leaking hundreds of thousands of them. This is too bad, since there are all sorts of interesting things you could do by using anonymous methods recursively. And since anonymous methods don’t have a name, they can’t invoke themselves recursively without capturing the reference variable.
There’s probably no way for the compiler to prevent this in the general case, but it could be solved easily enough for examples that fit the pattern described in my examples: When an anonymous method reference variable is only assigned once in an enclosing procedure, and that one point is where the method is defined within the procedure (i.e. it’s not passed in as a parameter or already assigned at global or object scope), then the compiler can be certain that the Self pointer for the underlying object is the same as the one it’s currently executing on, and transform it behind the scenes from an interface reference to a normal method call.
Can anyone else think of possible ways to fix this?
We’ve been aware of this issue with recursive anonymous method calls since the time they were designed. The problem comes from the limits of analysis: the call to the anonymous method is through a method reference location which is actually unassigned at the time of anonymous method construction – it’s only assigned after the construction has completed and the assignment occurs. After that point, if the method reference location is not modified, it will indeed leak – but that requires figuring out when, where and if a given location will be modified. How do we know it’s assigned once and never reassigned or cleared by the programmer? There could be an alias (a different name for the same location, such as a pointer), or it could be assigned multiple times (such as in a loop). Follow the analysis to its logical conclusion and you have to solve the Halting problem.
At best, the compiler could give a warning that it looks like you’re capturing the activation record inside the activation record itself. Or there could be some trickery done in the RTL, tracing through the object graph created by activation records, to automatically discover cycles at IInterface._Release time. Likely a runtime solution would be more robust and reliable than a compile-time one.
Don’t how to correctly pass our posts from that post, I’ll try with newgroups style 🙂
>> The solution to the memory leak http://stackoverflow.com/questions/6273376/memory-leaks-happens-in-nested-anonymous-method#18002857
>> looks promising from my pov. I added a comment about this at QC83259.
> That fix looks like it will work as long as you don’t pass either of the anonymous methods generated in the
> procedure outside if it to be used at some point after the procedure returns. Otherwise you’ll end up with
> access violations. But it’s a good workaround.
Actually it’s interesting, I tested it, no AV appears and it seems just because when an interface assigned to another one, the storage entry is different. The illustration is below. RefProc and fSavedCall, both are entries for interfaces. RefProc := Nil decreases the counter, but the object is not freed because fSavedCall still holds the reference to the same TInterfacedObject. I looked in the debugger, the actual Destroy-on-Counter-is-zero appears at the “fSavedCall:=Nil;” line. And ReportMemoryLeaksOnShutdown check passes with this test. Probably I’m wrong, but I could not invent a test when it doesn’t work.
…
type
TSomeRef = reference to procedure();
..
TForm16 = class(TForm)
private
fSavedCall: TSomeRef;
..
end;
…
procedure TForm16.Button6Click(Sender: TObject);
begin
PreparedTest;
fSavedCall();
fSavedCall:=Nil;
end;
procedure TForm16.PreparedTest;
var
StTick: Cardinal;
i: integer;
RefProc: TSomeRef;
AValue: integer;
begin
StTick:=timeGetTime;
AValue:=0;
RefProc:=procedure()
begin
Inc(AValue);
end;
for i:=0 to ManySteps-1 do
CallRefProc(procedure()
begin
// CallRefProc(RefProc);
PostponeCall(RefProc);
end);
RefProc:=Nil;
end.
Sorry for the formatting, don’t know how to force pre here and also forget the following lines
procedure TForm16.PostponeCall(const Call: TSomeRef);
begin
fSavedCall:=Call;
end;
procedure CallRefProc(const RefProc: TSomeRef);
begin
RefProc();
end;