aehimself 399 Posted August 14, 2020 A long time ago I wrote an application where I was forced to push specific long-running operations into background threads to keep the UI responsive. Stupidly enough, each method wrapper kept the control away from the VCL thread and returned only when the background process finished. This was a quick and dirty solution so not much of the code had to be changed. For example: myclass := TMyClass.Create; // Setup myclass // Instead of myclass.DoWork, now it looks like this: WorkWithBackgroundThread(myclass); // Get the results out of myclass FreeAndNil(myclass); Where in WorkWithBackgroundThread lies the evil: workthread := TWorkThread.Create; // setup workthread workthread.Start; While Not workthread.Finished Do Begin Application.ProcessMessages; Sleep(10); End; This worked for a while, up until I updated a component the application is using and now it started to throw invalid pointer operations. Long story short, the execution pointer / counter jumps in-and-out of different methods called by window messages, corrupting some Strings in the process (in the debugger the value quickly changes to "inaccessible value") so yeah... I am thinking on re-writing the whole thing to be event driven, but unfortunately far too many logic already depend on not receiving the control back. Is there a way doing something like this in modern Delphi (10.2+) ? I noticed that now we have a System.Threading unit with some task implementations, but ITask.Wait also blocks the caller thread (and this unit does not have a high reputation as far as comments went). The other bad thing about these implementations is exceptions and freeing an object up: Var sl: TStringList; x: ITask; begin sl := TStringList.Create; Try Try x := TTask.Create(Procedure Begin Sleep(10000); sl.Add('eeeee'); End); task.Start; // task.Wait; // will cause the UI to hang up Except On E:Exception Do ShowMessage(E.Message); End; Finally FreeAndNil(sl); End; Which will throw a nice and cozy nullpointer exception and is not getting caught in the except block... see my issue I suppose. So my question is: is there an easier salvation for me rather than the full refactor? I'd prefer not to use any 3rd party libraries to keep external dependency as low as possible. Share this post Link to post
Uwe Raabe 2064 Posted August 14, 2020 Var sl: TStringList; begin sl := TStringList.Create; TTask.Run( Procedure Begin Sleep(10000); sl.Add('added in Task thread'); TThread.Queue(nil, Procedure begin { this runs in the MainThread } sl.Free; end; End); end; Share this post Link to post
aehimself 399 Posted August 14, 2020 Yes, this will definitely get rid of the nullpointer but will still "swallow" all exceptions happening inside. During the past years I also grew to feel uncomfortable creating a local class without a Try...Finally block; however sometimes there's no way around. There is an other issue I forgot to mention - nested calls. Procedure1 calling procedure2, which starts a task and returns immediately. The issue is, Procedure1 will expect the data/action to be ready as it regains the program flow. So I guess there's no easy solution; I reap what I sow. One has to pay for his former mistakes 🙂 Ps: I never really liked this implementation anyway. Share this post Link to post
FredS 138 Posted August 14, 2020 25 minutes ago, aehimself said: guess there's no easy solution Either write a method which uses MsgWaitForMultipleObjects and also calls synchronize `WaitAsync` or something like that, or just use a short timeout for `TTask.Wait` and call `Application.ProcessMessages` while not terminated.. Share this post Link to post
aehimself 399 Posted August 14, 2020 Just now, FredS said: `WaitAsync` or something like that I'm completely new to System.Threading (I was told it exists today by one of my colleagues) and always used TThread descendants until now. I'll see what exactly this method does; thank you for the suggestion! 2 minutes ago, FredS said: or just use a short timeout for `TTask.Wait` and call `Application.ProcessMessages` while not terminated.. The main purpose of this change is to get rid of the Application.ProcessMessages call in a loop, so this is not an option unfortunately. Share this post Link to post
FredS 138 Posted August 14, 2020 54 minutes ago, aehimself said: Application.ProcessMessages call in a loop Start with this, I pulled that out of more complex code but haven't tested it: //MMWIN:MEMBERSCOPY unit _MM_Copy_Buffer_; interface type TThreadHelper = class helper for TThread public /// <summary> /// Will handle all Wait Key, Mouse and Message Inputs, also calls Synchronize as needed. /// </summary> function StartAsync: TWaitResult; end; implementation function TThreadHelper.StartAsync: TWaitResult; var LHandles: TArray<THandle>; Rsl : Cardinal; const cObjects = 2; begin Assert(TThread.CurrentThread.ThreadID = MainThreadID, 'Must be called from Main Thread'); Self.FreeOnTerminate := False; SetLength(LHandles, cObjects); LHandles[0] := Self.Handle; LHandles[1] := SyncEvent; self.Start(); try repeat Rsl := MsgWaitForMultipleObjects(cObjects, LHandles[0], False, INFINITE, QS_ALLINPUT); case Rsl of WAIT_FAILED: Exit(TWaitResult.wrError); WAIT_TIMEOUT: Exit(TWaitResult.wrTimeOut); WAIT_ABANDONED_0 .. WAIT_ABANDONED_0 + cObjects: Exit(TWaitResult.wrAbandoned); WAIT_OBJECT_0 : Result := TWaitResult.wrSignaled; WAIT_OBJECT_0 + 1: CheckSynchronize(0); WAIT_OBJECT_0 + cObjects: Application.ProcessMessages; end; until (Rsl = 0); finally self.Free; end; end; end. Share this post Link to post
Fr0sT.Brutal 900 Posted August 17, 2020 On 8/14/2020 at 9:56 PM, aehimself said: A long time ago I wrote an application where I was forced to push specific long-running operations into background threads to keep the UI responsive On 8/14/2020 at 9:56 PM, aehimself said: unfortunately far too many logic already depend on not receiving the control back Two controversial sentences, as for me. If UI is responsible, user can do some actions. If logic depends on receiving the control back, user is not supposed to do things that could change app's state while bg operation is running. So I guess the easiest workaround would be to block any actions from UI that could interfere with bg task (you should do it anyway even if bg task won't use App.ProcMsgs). Like with shared variables, app's state must be isolated from simultaneous change. Share this post Link to post
aehimself 399 Posted August 17, 2020 (edited) 11 hours ago, Fr0sT.Brutal said: Two controversial sentences, as for me. If UI is responsible, user can do some actions. If logic depends on receiving the control back, user is not supposed to do things that could change app's state while bg operation is running. The UI is a simple tabbed interface. One tab can be "blocked" (by disabling all buttons and actions when a BG operation starts; re-enabled when ends) but the others (and with that - the whole UI) must stay unblocked. Furthermore, a nice spinning loading indicator with a "Cancel" button looks and feels more "professional" than a frozen, whitened form what you only can close with the task manager. Edit: As I already took a shortcut with the While xx Do Application.ProcessMessages method and seeing where it lead to - I am already in the process of rewriting the whole "action" logic to pure event based. It's a nightmare. To any future reader: don't be like me. Write your code clean, so you have to write it only once 🙂 Edited August 17, 2020 by aehimself Share this post Link to post
Vincent Parrett 765 Posted August 17, 2020 I wrote https://github.com/VSoftTechnologies/VSoft.Awaitable for this purpose.. in a tabbed interface where each tab invokes a background task to fetch data, but switching tabs in the middle of the request is allowed (and starting a new request, fetching a different set of data). My library is a wrapper over Omnithread Library which just makes it easier to cancel tasks and to return values. TAsync.Configure<string>( function (const cancelToken : ICancellationToken) : string var i: Integer; begin result := 'Hello ' + value; for i := 0 to 2000 do begin Sleep(1); //in loops, check the token if cancelToken.IsCancelled then exit; end; //where api's can take a handle for cancellation, use the token.handle WaitForSingleObject(cancelToken.Handle,5000); //any unhandled exceptions here will result in the on exception proc being called (if configured) //raise Exception.Create('Error Message'); end, token); ) .OnException( procedure (const e : Exception) begin //runs in main thread Label1.Caption := e.Message; end) .OnCancellation( procedure begin //runs in main thread //clean up Label1.Caption := 'Cancelled'; end) .Await( procedure (const value : string) begin //runs in main thread //use result Label1.Caption := value; end); So when the user switches tabs, or to another part of the UI, we can cancel the background task via the CancellationToken (see https://github.com/VSoftTechnologies/VSoft.CancellationToken) and start a new one. Of course being able to cancel what is happening in your thread relies on whatever you are calling being able to be cancelled or aborted. I wrote my own http client (https://github.com/VSoftTechnologies/VSoft.HttpClient) to ensure I could cancel requests. Also, CancellationTokens have a Handle that can be passed to WaitForMultipleObjects or api's that take event handles for cancellation. Share this post Link to post
Fr0sT.Brutal 900 Posted August 18, 2020 11 hours ago, aehimself said: The UI is a simple tabbed interface. One tab can be "blocked" (by disabling all buttons and actions when a BG operation starts; re-enabled when ends) but the others (and with that - the whole UI) must stay unblocked. Furthermore, a nice spinning loading indicator with a "Cancel" button looks and feels more "professional" than a frozen, whitened form what you only can close with the task manager. Surely I understand; but you have to guarantee that data used by bg task won't be modified by actions from UI, regardless of the architecture you use (threads with events or threads with ProcessMsgs). Share this post Link to post