0

So I came across some code similar to this:

aThread := TmyThread.Create(param1, param2);
h := aThread.handle;
repeat
  if (MsgWaitForMultipleObjects(1, h, False, INFINITE, QS_ALLINPUT) = WAIT_OBJECT_0)
    then break;
  Application.ProcessMessages
until False;

aThread.Free;
myProgressBar.Progress := myProgressBar.Max;  //an internal component, not really important here

Which, I assume, is meant to provide a way of updating the GUI so it doesn't appear blocked, but also allow for the end process GUI to be updated (the progress bar), while some long task is taking place.

But it contains the dreaded Application.ProcessMessages.

I've read The Darkside of Application.ProcessMessages and many other Delphi blogs suggesting it's time to use a new thread instead when Application.ProcessMessages is being used.

So, is it wise to phase out this method of keeping the main/GUI threaded idle for something like the AnonymousThread approach shown here? Or something else?

This Noob is confused as to why it's suggested that a process that calls Application.ProcessMessages is a good candidate for a Thread, but the thread in question relies on the very thing we're being told not to do!

Community
  • 1
  • 1
leftyloosie
  • 518
  • 4
  • 11
  • 3
    This code represents a situation where the main thread have to wait for the spawned thread to finish before continuing, for whatever it is doing... If all you need is indicating that the thread is finished, TThread has an OnTerminate event. – Sertac Akyuz Sep 27 '16 at 23:53
  • 3
    `TThread` also has a `WaitFor()` method that blocks the calling thread (while still processing `Queue`/`Synchronize` requests if called by the main thread) until the worker thread terminates. – Remy Lebeau Sep 28 '16 at 03:31
  • If you have to spawn a worker thread, it is best not to block the UI thread waiting on the worker thread at all. Start the thread and return control back to the main message loop. Let the worker thread notify the UI thread when it has status updates to display, or when it is finished with its work. – Remy Lebeau Sep 28 '16 at 03:37
  • Hard to say with no more context. But generally you avoid this issue by using an asynchronous approach and letting the thread signal that it is done. – David Heffernan Sep 28 '16 at 06:41
  • Thank you! I think in this particular case, the worker thread backs up a DB and the DB server required this be done in its own thread. Aside from this case, we are also looking for opportunities to move long processes out to worker threads to that synchronize with the UI (rather than call A.PM a lot during the long process), but also indicate when they are finished in some cases. – leftyloosie Sep 28 '16 at 14:45

1 Answers1

1

The main idea is not to wait for the thread. Thread should inform your form when it finished. In other words the code which should be executed after the the thread is finished should be isolated to a separate procedure (see TForm1.ThreadCompletedHandler) and thread should call it after it is finished.

Here is a small sample:

unit Unit1;

interface

uses
  Winapi.Windows, Winapi.Messages,
  System.SysUtils, System.Variants, System.Classes,
  Vcl.Graphics, Vcl.Controls, Vcl.Forms, Vcl.Dialogs, Vcl.StdCtrls, Vcl.ComCtrls;

type
  TmyThread = class(TThread)
  private
    FParam1, FParam2: Integer;
    FUpdateProc: TProc<TmyThread, Integer>;
    FCompleteProc: TProc<TmyThread>;

    procedure SyncCompleteProc;
    procedure QueueUpdateProc(APosition: Integer);
  protected
    procedure Execute; override;
  public
    constructor Create(AParam1, AParam2: Integer;
      AUpdateProc: TProc<TmyThread, Integer>;
      ACompleteProc: TProc<TmyThread>);
  end;

  TForm1 = class(TForm)
    Button1: TButton;
    myProgressBar: TProgressBar;
    procedure Button1Click(Sender: TObject);
  private
    FThread: TThread;

    procedure ThreadUpdateHandler(AThread: TMyThread; APosition: Integer);
    procedure ThreadCompletedHandler(AThread: TMyThread);
  protected
    procedure UpdateActions; override;
  public
    destructor Destroy; override;
  end;

var
  Form1: TForm1;

implementation

{$R *.dfm}

{ TmyThread }

constructor TmyThread.Create(AParam1, AParam2: Integer; AUpdateProc: TProc<TMyThread, Integer>;
  ACompleteProc: TProc<TMyThread>);
begin
  FParam1 := AParam1;
  FParam2 := AParam2;
  FUpdateProc := AUpdateProc;
  FCompleteProc := ACompleteProc;
  inherited Create(False);
end;

procedure TmyThread.Execute;
var
  I: Integer;
begin
  //inherited; - abstract
  try
    I := FParam1;
    while not Terminated and (I < FParam2) do
    begin
      Sleep(1000);
      Inc(I);
      QueueUpdateProc(I);
    end;
  finally
    if Assigned(FCompleteProc) then
      TThread.Queue(Self, SyncCompleteProc);
  end;
end;

procedure TmyThread.QueueUpdateProc(APosition: Integer);
begin
  if Terminated or not Assigned(FUpdateProc) then
    Exit;

  TThread.Queue(Self,
    procedure
    begin
      FUpdateProc(Self, APosition);
    end);
end;

procedure TmyThread.SyncCompleteProc;
begin
  FCompleteProc(Self);
end;

{ TForm1 }

procedure TForm1.Button1Click(Sender: TObject);
const
  param1 = 1;
  param2 = 5;
begin
  myProgressBar.Min := param1;
  myProgressBar.Max := param2 + 1;
  myProgressBar.Position := param1;

  FThread := TmyThread.Create(param1, param2, ThreadUpdateHandler, ThreadCompletedHandler);
end;

destructor TForm1.Destroy;
begin
  if Assigned(FThread) then
    FThread.Terminate;

  inherited;
end;

procedure TForm1.ThreadCompletedHandler(AThread: TmyThread);
begin
  try
    if not AThread.Terminated then // check form is not destroye yet
    begin
      FThread := nil;
      myProgressBar.Position := myProgressBar.Max;  //an internal component, not really important}
    end;
  finally
    FreeAndNil(AThread);
  end;
end;

procedure TForm1.ThreadUpdateHandler(AThread: TMyThread; APosition: Integer);
begin
  if not AThread.Terminated then // check form is not destroye yet
    myProgressBar.Position := APosition;
end;

procedure TForm1.UpdateActions;
begin
  inherited;
  Button1.Enabled := not Assigned(FThread);
end;

end.

and DFM file

object Form1: TForm1
  Left = 0
  Top = 0
  Caption = 'Form1'
  ClientHeight = 290
  ClientWidth = 554
  Color = clBtnFace
  Font.Charset = DEFAULT_CHARSET
  Font.Color = clWindowText
  Font.Height = -11
  Font.Name = 'Tahoma'
  Font.Style = []
  OldCreateOrder = False
  PixelsPerInch = 96
  TextHeight = 13
  object Button1: TButton
    Left = 24
    Top = 16
    Width = 75
    Height = 25
    Caption = 'Button1'
    TabOrder = 0
    OnClick = Button1Click
  end
  object myProgressBar: TProgressBar
    Left = 120
    Top = 108
    Width = 150
    Height = 17
    TabOrder = 1
  end
end

NB In this sample form does not wait for the thread so potentially we may close the application before the thread is terminated. It should not be a problem for this sample as it simple enough but in real live you may need to wait for the thread in Form.Destroy or create some thread manager which should wait for all running threads before application is finished.

Max Abramovich
  • 473
  • 4
  • 11
  • Thank you for such a great example. While I understand you are providing the better architecture, I think I was looking for an answer on the lines of: if one MUST wait on a thread (for instance, we have a long process, and we don't want our users to click around and get into anything else until the long process is complete) is myThread.waitFor better than the code provided above? – leftyloosie Dec 12 '16 at 20:52
  • Hm, you are using Application.ProcessMessages so all user clicks will be processed. If you do not want it you should replace ProcessMessages with Update method (it will just redraw all the controls if needed). And there is no need to create any other threads if you just want to block the application (except redraw) till the thread is finished – Max Abramovich Dec 14 '16 at 21:27