Thread not executing on open non-modal form

Question

Below is part of the code for a 'progress' form.
Apart from ProgressBars (removed from code) it has a TLabel (LblDots) of which I want to change the caption (number of dots increasing).
In the FormShow/FormClose the TDotterThread gets created and destroyed.

Problem:
I see the Synchronize(DoUpdate) procedure that updates the label only being called when the program is not doing heavy work.

This is the progress form:

unit FrmBusy;

interface

uses
   System.SyncObjs, Windows, Messages, SysUtils, System.Classes, Graphics, Controls, Forms, Dialogs, StdCtrls;

type
   TUpdateEvent = procedure of object;    // 'of object' to prevent 'Incompatible types: regular procedure and method pointer'

type
   TDotterThread = class(TThread)         // Thread to update LblDots
   private
      FTick: TEvent;
      FUpdater: TUpdateEvent;
   protected
      procedure Execute; override;
      procedure DoUpdate;
   public
      constructor Create;
      destructor Destroy; override;
      property Updater: TUpdateEvent read FUpdater write FUpdater;
      procedure Stop;
   end;

type
  TFormBusy = class(TForm)
    LblDots: TLabel;
    procedure FormShow(Sender: TObject);
    procedure FormClose(Sender: TObject; var Action: TCloseAction);
  private
    FShowDots: Boolean;
    FDotterThread: TDotterThread;
    procedure UpdateDots;
  public
    property ShowDots: Boolean write FShowDots;
  end;

implementation

{$R *.DFM}

procedure TFormBusy.FormClose(Sender: TObject; var Action: TCloseAction);
begin
   if FShowDots then FDotterThread.Stop; // Calls Terminate and is FreeOnTerminate
end;

procedure TFormBezig.UpdateDots;
var s: String;
begin
   s := LblDots.Caption;
   if Length(s) = 50 then s := '' else s := s + '.';
   LblDots.Caption := s;
   Application.ProcessMessages;
end;

procedure TFormBusy.FormShow(Sender: TObject);
begin
   LblDots.Caption := '';
   if FShowDots then
   begin
      FDotterThread := TDotterThread.Create;
      FDotterThread.Updater := Self.UpdateDots;
      FDotterThread.Start;
   end;
   BringWindowToTop(Self.Handle);
end;

{ TDotterThread }

constructor TDotterThread.Create;
begin
  FTick := TEvent.Create(nil, True, False, '');
  FreeOnTerminate := true;
  inherited Create(true);  // Suspended
end;

destructor TDotterThread.Destroy;
begin
  FTick.Free;
  inherited;
end;

procedure TDotterThread.DoUpdate;
begin
   if Assigned(FUpdater) then FUpdater;
end;

procedure TDotterThread.Execute;
begin
  while not Terminated do
  begin
     FTick.WaitFor(1000);
     Synchronize(DoUpdate);
  end;
end;

procedure TDotterThread.Stop;
begin
   Terminate;
   FTick.SetEvent;
end;

end.

The form is called and created like:

procedure TFrmTest.FormCreate(Sender: TObject);
begin
  FFormBusy := TFormBusy.Create(nil);
end;

procedure TFrmTest.FormDestroy(Sender: TObject);
begin
   FFormBusy.Free;
end;

procedure TFrmTest.BtnCompareClick(Sender: TObject);
begin
   FrmTest.FFormBusy.ShowDots := true;
   FrmTest.FFormBusy.Show;
   FrmTest.FFormBusy.Update label/progress bar
   DoHeavyWork1();
   FrmTest.FFormBusy.Update label/progress bar
   DoHeavyWork2();
   etc.
end;

What am I doing wrong?
TIA

since DoUpdate has to be called synchronized, you are in the context of the mainthread and your update procedure can only be processed if the mainthread has "idle time". Maybe you can change it to process DoHeavyWorkx within a thread. — bummi, May 08 '13 at 08:41
@bummi is correct - by synchronizing the method you are just pushing it back onto the main thread and defeating the entire purpose. I think the important procedure to show us, and the one you need to modify, is `FFormBusy.UpdateDots` - could you post that code? (show more dots... more dots!) — J..., May 08 '13 at 11:01
@J I see I accidenttaly cut that out of the code - edited my question to fix that. — Jan Doggen, May 08 '13 at 11:03

David Heffernan · Accepted Answer · 2013-05-08T12:48:56.797

As you know, all UI code must be executed on the main GUI thread. That's why you are calling Synchronize to update your GUI. Synchronize works broadly like this:

The task to be performed on on the main thread is placed in a queue.
The main thread is signaled to indicate that synchronize tasks are pending.
The background thread blocks.
When the main thread next checks to see if there are pending synchronize tasks, it executes them.
The background thread is signaled to indicate that the task has been executed.
The background thread stops blocking and continues executing.

It's quite a complex little dance.

Your problem is that your main thread is busy executing some long running task. Presumably in the calls to DoHeavyWork1 and DoHeavyWork2. And that means that the GUI thread does not perform item 4 in a timely fashion. What's more, the main thread blocks the background thread, somewhat negating the utility of threads.

Your problem, fundamentally, is that your main GUI thread is busy doing something other than servicing the GUI. You should dedicate your GUI thread to servicing the GUI. It should take on nothing else, and certainly not any long running tasks. Once you manage to ship all the non-GUI tasks out of the GUI thread and onto background threads, you'll find that you application is responsive.

Finally, I recommend that you remove that call to Application.ProcessMessages from UpdateDots. You probably added it to try to deal with your non-responsive GUI. But it won't help at all because your problem is that UpdateDots is not executing in a timely fashion.

Perhaps to sum it up succinctly - he should be putting `DoHeavyWork` into the TThread rather than trying to marshall UI updates from a worker thread. Occasionally calling `Synchronize` to update the UI from within `DoHeavyWork` running on a TThread would work. — J..., May 08 '13 at 12:46
@J... Marshalling UI updates from a worker thread is fine so long as the GUI thread is responsive and available to service them. — David Heffernan, May 08 '13 at 12:49
Naturally. I see how that doesn't quite read right now. I meant to contrast the wholesale offloading of the UI task (and *only* the UI work) to a worker thread, only to be marshalled back again, vs offloading of the heavy work. — J..., May 08 '13 at 17:01
"As you know," indeed, only that had sunk to the bottom of my brain ;-) Thanks, bummi too. — Jan Doggen, May 08 '13 at 17:53

Thread not executing on open non-modal form

1 Answers1