Jump to content
aehimself

While Not _thread.Finished Do Application.ProcessMessages;

Recommended Posts

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
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

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
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
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
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
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
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 by aehimself

Share this post


Link to post

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
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

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

×