Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Closure in TTask.Run(AnonProc) not released after AnonProc has finished

Anonymous methods in Delphi create a closure, which keeps "surrounding" local variables in context, until the anonymous method has finished. If using interface variables, then they will decrease their referenced instance not before the anonymous method has finished. So far so good.

When using TTask.Run(AProc:TProc) with an anonymous method I would expect that the closure will be released when the associated worker thread has finished executing "AProc". This does not seem to happen though. On program termination, when the thread pool (that this TTask generated thread belongs to) gets released, you can finally see that these locally referenced instances get released - i.e. the closure gets apparently released.

The question is if this is a feature or a bug? Or do I oversee something here?

Below, after TTask.Run(...).wait I would expect LFoo's destructor to be called - which does not happen.

procedure Test3;
var
  LFoo: IFoo;
begin
  LFoo := TFoo.Create;

  TTask.Run(
    procedure
    begin
      Something(LFoo);
    end).Wait; // Wait for task to finish

   //After TTask.Run has finished, it should let go LFoo out of scope - which it does not apprently. 
end;

Following is a full test case, which shows that a "simple" anonymous method works as expected (Test2), but when fed into TTask.Run it does not (Test3)

program InterfaceBug;

{$APPTYPE CONSOLE}
{$R *.res}

uses
  System.Classes,
  System.SysUtils,
  System.Threading;

type

  //Simple Interface/Class
  IFoo = interface(IInterface)
    ['{7B78D718-4BA1-44F2-86CB-DDD05EF2FC56}']
    procedure Bar;
  end;

  TFoo = class(TInterfacedObject, IFoo)
  public
    constructor Create;
    destructor Destroy; override;
    procedure Bar;
  end;

procedure TFoo.Bar;
begin
  Writeln('Foo.Bar');
end;

constructor TFoo.Create;
begin
  inherited;
  Writeln('Foo.Create');
end;

destructor TFoo.Destroy;
begin
  Writeln('Foo.Destroy');
  inherited;
end;

procedure Something(const AFoo: IFoo);
begin
  Writeln('Something');
  AFoo.Bar;
end;

procedure Test1;
var
  LFoo: IFoo;
begin
  Writeln('Test1...');
  LFoo := TFoo.Create;
  Something(LFoo);
  Writeln('Test1 done.');
  //LFoo goes out od scope, and the destructor gets called
end;

procedure Test2;
var
  LFoo: IFoo;
  LProc: TProc;
begin
  Writeln('Test2...');
  LFoo := TFoo.Create;
  LProc := procedure
    begin
      Something(LFoo);
    end;
  LProc();
  Writeln('Test2 done.');
   //LFoo goes out od scope, and the destructor gets called
end;

procedure Test3;
var
  LFoo: IFoo;
begin
  Writeln('Test3...');
  LFoo := TFoo.Create;
  TTask.Run(
    procedure
    begin
      Something(LFoo);
    end).Wait; // Wait for task to finish
  //LFoo := nil;  This would call TFoo's destructor,
  //but it should get called automatically with LFoo going out of scope - which apparently does not happen!
  Writeln('Test3 done.');
end;

begin
  try
    Test1; //works
    Writeln;
    Test2; //works
    Writeln;
    Test3; //fails
    Writeln('--------');
    Writeln('Expected: Three calls of Foo.Create and three corresponding ones of Foo.Destroy');
    Writeln;
    Writeln('Actual: The the third Foo.Destroy is missing and is executed when the program terminates, i.e. when the default ThreadPool gets destroyed.');
    ReadLn;
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;

end.
like image 699
Olaf Monien Avatar asked Nov 17 '16 11:11

Olaf Monien


2 Answers

I did some more analysis of this bug to find out the real reason why the ITask was being held in TThreadPool.TQueueWorkerThread.Execute as mentioned in the known issue.

The following innocent looking line of code is the problem:

Item := ThreadPool.FQueue.Dequeue;

Why is that the case? Because TQueue<T>.Dequeue is marked as inline and now you have to know that the compiler does not apply the so called return value optimization for inlined functions returning a managed type.

This means the line before really gets translated (I very much simplified this) into this code by the compiler. tmp is a compiler generated variable - it reserves space on the stack in the prologue of the method:

tmp := ThreadPool.FQueue.Dequeue;
Item := tmp;

This variable gets finalized in the end of the method. You can put a breakpoint there and one into TTask.Destroy and then you see that when the application ends once it reaches the end of the method this will trigger the last TTask instance getting destroyed because the temp variable keeping it alive is being cleared.

I used some little hack to fix this issue locally. I added this local procedure to eliminate the temporary variable sneaking into the TThreadPool.TQueueWorkerThread.Execute method:

procedure InternalDequeue(var Item: IThreadPoolWorkItem);
begin
  Item := ThreadPool.FQueue.Dequeue;
end;

and then changed the code inside the method:

InternalDequeue(Item);

This will still cause the Dequeue to produce a temporary variable but now this only lives inside the InternalDequeue method and is being cleared once it exits.

Edit (09.11.2017): This has been fixed in 10.2 in the compiler. It now inserts a finally block after the assignment of the temp variable to the real one so the temp variable does not cause an additional reference any longer than it should.

like image 105
Stefan Glienke Avatar answered Nov 18 '22 21:11

Stefan Glienke


This is known issue: TThreadPool worker thread holds reference to last executed task

A temporary variable in TThreadPool.TQueueWorkerThread.Execute keeps a reference to the last executed work-item (task), which is only released when the Execute method ends.

Being in a pool the thread is usually kept alive until the pool is destroyed, which for the Default pool means during finalization of the unit. Thus the last executed tasks are not released until the program terminates.

like image 43
Dalija Prasnikar Avatar answered Nov 18 '22 21:11

Dalija Prasnikar