Jump to content
FabDev

Thread and free

Recommended Posts

Hello,

 

What is the better code to free a shared resource between thread (lStrings.free) :
 

procedure TForm16.ShowComputedDetail;
begin
TTask.run(
   procedure
   var
     lTstrings:Tstrings;
   begin
     lTstrings:=Tstringlist.create;
     try
         ComputeDetail(lTstrings);
         TThread.Queue(nil,
         procedure
         begin
             MyTMemo.BeginUpdate;
             MyTMemo.lines.Assign(lTstrings);
             MyTMemo.EndUpdate;
             lTstrings.Free;
         end);
     finally

     end;
   end);
end;

 

or

 

procedure TForm16.ShowComputedDetail;
begin
TTask.run(
   procedure
   var
     lTstrings:Tstrings;
   begin
     lTstrings:=Tstringlist.create;
     try
         ComputeDetail(lTstrings);
         TThread.Queue(nil,
         procedure
         begin
             MyTMemo.BeginUpdate;
             MyTMemo.lines.Assign(lTstrings);
             MyTMemo.EndUpdate;
             
         end);
     finally
          lTstrings.Free;
     end;
   end);
end;

 

 

 

 

Edited by FabDev

Share this post


Link to post

The first will not ensure that lTstrings will be freed.

I would use the second but with TThread.Synchronize() because Queue() will continue the execution and lTstrings will be freed most probably before it will be used.

Share this post


Link to post

In the Queue procedure you are using resources allocated in the Anonymous TASK, I don't think it is correct to "free" them that way: when references to lTstrings in the Main Thread are used (asynchronously from your try / finally) these will have to exist... and you however, you "free" them sequentially to your program flow.

I honestly don't know how to resolve the situation with the example you gave.

However, I could have made a mistake in the analysis, but I am still curious about the comment of someone who is certainly more expert than me.

Bye

 

@Cristian Peța

sorry for duplicate, we have write at the same time.

Edited by DelphiUdIT

Share this post


Link to post
1 hour ago, FabDev said:

What is the better code to free a shared resource between thread (lStrings.free) :

The first example is the correct one. The second one is not, as TThread.Queue will run after lStrings instance is being destroyed. Also it is extremely important that you pass nil to the TThread.Queue like you did because passing the current thread instance TThread.Queue(TThread.CurrentThread, would remove queued procedure when the thread has finished running and if that happens before queued code had the chance to run there would be a memory leak. This is less of a problem with tasks, as their threads are reused, although theoretically, such situation can still happen. But if you replace TTask with anonymous or custom thread then such situation is more likely to happen, especially if there are no more code in a thread after calling Queue that could delay its destruction. 

 

However, the second example would also work if you would use TThread.Synchronize instead of Queue ad Synchronize is blocking call and Free will be executed only after code inside Synchronize is executed. 

  • Thanks 1

Share this post


Link to post

If the question is: "Which is better: Version A.) or B.) ( with blocking TThread.Synchronize, of course )"

Then I would tend to B.), because it's more "local" and keeping the thread running until it's really finished.

Version A.) has way more options to go terrible wrong, IMHO.

 

The question is, if there would be really a use-case for A.), to leave the thread, before it's completely finished?

 

I'm not sure if A.) would allow re-using the thread, before the Queue has completely finished and cleaned up.

For example, if the Queue would block for 5 seconds, could then the thread re-used elsewhere?

 

I doubt that it can be re-used without headaches, because the variables were captured and perhaps the whole anonymous thread space too, until it completely went out of scope.

The capture mechanics must capture the thread and the lTstrings variable as well, so I would assume that both capture spaces were not 100% independent.

 

 

Edited by Rollo62

Share this post


Link to post

TThread.Queue() runs asynchrously - it returns to the caller immediately and its passed procedure is executed in the main thread at some future time.  So, your first example is mostly correct - the anonymous procedure will have to transfer ownership of the TStringList to the main thread so it can free the TStringList after updating the UI.  However, be sure to move your try..finally into the procedure as well to ensure the TStringList is actually freed even if something goes wrong, eg:

procedure TForm16.ShowComputedDetail;
begin
  TTask.Run(
    procedure
    var
      lTstrings: TStrings;
    begin
      lTstrings := TStringList.create;
      try
        ComputeDetail(lTstrings);
        TThread.Queue(nil,
          procedure
          begin
            try
              MyTMemo.BeginUpdate;
              try
                MyTMemo.Lines.Assign(lTstrings);
              finally
                MyTMemo.EndUpdate;
              end;
            finally
              lTstrings.Free;
            end;
          end);
      except
        lTstrings.Free;
        raise;
      end;
    end);
end;

In your 2nd example, you will need to change TThread.Queue() to TThread.Synchronize() to ensure the TStringList is not freed before the main thread has a chance to use it, eg:

procedure TForm16.ShowComputedDetail;
begin
  TTask.Run(
    procedure
    var
      lTstrings: TStrings;
    begin
      lTstrings := TStringList.create;
      try
        ComputeDetail(lTstrings);
        TThread.Synchronize(nil,
          procedure
          begin
            MyTMemo.BeginUpdate;
            try
              MyTMemo.Lines.Assign(lTstrings);
            finally
              MyTMemo.EndUpdate;
            end;
          end);
      finally
        lTstrings.Free;
      end;
    end);
end;

 

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×