FabDev 8 Posted October 27, 2023 (edited) 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 October 27, 2023 by FabDev Share this post Link to post
Cristian Peța 107 Posted October 27, 2023 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
DelphiUdIT 187 Posted October 27, 2023 (edited) 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 October 27, 2023 by DelphiUdIT Share this post Link to post
Dalija Prasnikar 1404 Posted October 27, 2023 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. 1 Share this post Link to post
Rollo62 539 Posted October 27, 2023 (edited) 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 October 27, 2023 by Rollo62 Share this post Link to post
Remy Lebeau 1436 Posted October 27, 2023 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