Forum > General

[SOLVED] Thread Safety?

(1/13) > >>

knuckles:
I want to learn how to use TThreads so the UI doesn't lock up when running cpu intensive tasks, below I made my first attempt at creating a thread:


--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---unit Unit1; {$mode objfpc}{$H+} interface uses  Classes, SysUtils, FileUtil, Forms, Controls, Graphics, Dialogs, StdCtrls,  ComCtrls; type  TForm1 = class(TForm)    Button1: TButton;    Label1: TLabel;    ProgressBar1: TProgressBar;    procedure Button1Click(Sender: TObject);  private    //  public    //  end;   TMyThread = class(TThread)  private    FProgress: Integer;  protected    procedure Execute; override;    procedure UpdateProgress;  end; var  Form1: TForm1; implementation {$R *.lfm} { TMyThread } procedure TMyThread.Execute;var  I: Integer;begin  for I := 0 to 25000 do  begin    FProgress := I;    Synchronize(@UpdateProgress);  end;end; procedure TMyThread.UpdateProgress;begin  Form1.ProgressBar1.Position := FProgress; // not thread safe?  Form1.Label1.Caption := FProgress.ToString; // not thread safe?end; { TForm1 } procedure TForm1.Button1Click(Sender: TObject);var  MyThread: TMyThread;begin  ProgressBar1.Max := 25000;  ProgressBar1.Position := 0;  Label1.Caption := '0';    MyThread := TMyThread.Create(False);end; end.
I'm not sure if I am doing this wrong though, I hear about thread safety with the VCL (or in this case LCL) and I look at my procedure UpdateProgress and I directly access controls on Form1 - Isn't this incorrect, and if so what changes should I make to ensure it is thread safe and won't crash?

Currently the program creates the thread and it runs without error when I click the button, it feels a little jittery but does seem to work, however I am unsure if my approach is correct or flawed?

Many thanks in advance :)

taazz:
yes and no, yes this is one way to safely update your form. no, you got some details wrong.
1) do not use any variables that you do not have direct control from any thread code, especially globals, aka stop using the Form1 variable, it might not update what you expect and have a crash.
2) Synchronise, although perfectly valid choice, is the worst choice you have if updates are rapid, it might be preferable to use Application.QueueAsyncCall just make sure that the call you queue is a method of the form you update not the thread and you are as safe as you can be.

rvk:
Overall your code seems fine.

In addition to the remarks taazz already gave you need to realize that Synchronize() is called a lot of times. Besides using Application.QueueAsyncCall you could also use TThread.Queue in which case the call to your UpdateProgress is queued.

But calling UpdateProgress so many times (25000 in a short time) isn't really needed, is it? Why not call UpdateProgress only every 10th (or 100th) iteration? You won't see every number flashing on screen anyway (unless you have superhuman vision). So be careful when to call Synchronize, Application.QueueAsyncCall or TThread.Queue and don't call it a few hundred times a second.

knuckles:
Thanks for the replies and information, my code was just a test, I will try and read more of what has been said and see if I can update it more appropriately in the next day or so and will come back hopefully with something that looks more correct and doesn't use Form1

zoltanleo:
@knuckles

I will make bold to give advice that you have studied the mechanism of sending messages to the main thread by means of SendMessage/PostMessage. It will allow you to operate operation of application more flexibly. Then your code will look approximately so

--- Code: Pascal  [+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i < x.length; i++) { x[i].style.maxHeight='none'; x[i].style.height = Math.min(x[i].clientHeight+15,306)+'px'; x[i].style.resize = "vertical";}};} ---unit Unit1; {$mode objfpc}{$H+} interface uses  Classes, SysUtils, Forms, Controls, Graphics, Dialogs, ComCtrls, StdCtrls  , LCLIntf, LCLType, LMessages, LResources; //add this units const  LM_MYMSG = LM_USER + $101; type   { TMyThread }   TMyThread = class(TThread)  private    FThreadIsTerm: Boolean;  protected    procedure Execute; override;  public    constructor Create(CreateSuspended: Boolean);  end;   { TForm1 }   TForm1 = class(TForm)    Button1: TButton;    Label1: TLabel;    ProgressBar1: TProgressBar;    procedure Button1Click(Sender: TObject);    procedure LmMyMsg(var Msg: TLMessage); message LM_MYMSG;  private    FMyThread: TMyThread;    FProgress: Integer;    FCaption: String;  public   end; var  Form1: TForm1; implementation {$R *.lfm} { TForm1 } procedure TForm1.Button1Click(Sender: TObject);begin  if Assigned(FMyThread) then Exit;   ProgressBar1.Max := 250;  ProgressBar1.Position := 0;  Label1.Caption := '0';  Button1.Enabled:= False;  try    FMyThread:= TMyThread.Create(True);     while not FMyThread.FThreadIsTerm do    begin      ProgressBar1.Position:= FProgress;      Label1.Caption:= FCaption;      Application.ProcessMessages;      Sleep(50);    end;   finally    FreeAndNil(FMyThread);    ProgressBar1.Position := 0;    Label1.Caption := '0';    Button1.Enabled:= True;  end;end; procedure TForm1.LmMyMsg(var Msg: TLMessage);begin  FCaption:= String(Msg.lParam);  FProgress:= Msg.wParam;end; { TMyThread } procedure TMyThread.Execute;var  Msg: TLMessage;  i: Integer;begin  for i:= 0 to 250 do    begin      Msg.lParam:= NativeInt(IntToStr(i));//it's an artificial example, only for demonstration of transfer of a string      PostMessage(Form1.Handle,LM_MYMSG,WPARAM(i),LPARAM(Msg.lParam));      Sleep(50);    end;  FThreadIsTerm:= True;end; constructor TMyThread.Create(CreateSuspended: Boolean);begin  inherited Create(CreateSuspended);  FreeOnTerminate:= False;  Priority:= tpLower;  FThreadIsTerm:= False;  Start;end;end.                    This code cross-platform. He will work also for Linux also  ;)

Navigation

[0] Message Index

[#] Next page

Go to full version