3

I'm new at SO, so forgive me if my question isn't in the right place or been answered before. The questions is about multi-threading with Delphi 10.4. I'm getting Access Violation error on my app, here is a very simple example:

type
  myThread = class(TThread)
  protected
    procedure Execute; override;
  end;

  TForm1 = class(TForm)
    Button1: TButton;
    Memo1: TMemo;
    procedure Button1Click(Sender: TObject);
    procedure FormClose(Sender: TObject; var Action: TCloseAction);
  private
    { Private declarations }
    mySideTask : myThread;

  public
    { Public declarations }
  end;
  
var
  Form1: TForm1;

implementation

{$R *.dfm}

procedure TForm1.Button1Click(Sender: TObject);
begin
  with mySideTask.Create do
    FreeOnTerminate:=True
end;

procedure TForm1.FormClose(Sender: TObject; var Action: TCloseAction);
begin
  if mySideTask<>nil then
  begin
    mySideTask.Terminate;
    mySideTask.WaitFor;
    FreeAndNil(mySideTask);
  end;
end;

{ myThread }

procedure myThread.Execute;
begin
  Synchronize(
    procedure
    begin
        Form1.Memo1.Lines.Add('running my side task')
    end);
end;

No error if I don't create an instance of the thread (which is confusing me):

procedure TForm1.Button1Click(Sender: TObject);
begin
  myThread.Create
end;

Can you please let me know what am I missing.

Ray
  • 45
  • 3
  • Learn how to create objects correctly. First, types should be prefixed with `T`, which is the standard; `MyThread` should be `TMyThread`. Second, you need to create an instance of that object. `var MyThread: TMyThread; begin MyThread := TMyThread.Create;`. This applies to all class types, not just threads. You should find a good Delphi tutorial to learn the basics before you start trying to do more advanced things like multithreading. – Ken White Aug 07 '22 at 02:22
  • 3
    @KenWhite prefixing a type with `T` is just a convention, not a requirement. And in the code shown, `myThread.Create` is creating an object instance of the thread class. The only problem I see is the `mySideTask` variable is not being assigned, but is being accessed. – Remy Lebeau Aug 07 '22 at 03:23
  • @RemyLebeau: Where did I say it was a requirement? I said it was a standard, which is used in the Delphi source itself and by almost every Delphi programmer as a standard way of naming types, and I said the OP *should*, not *must*. Where does that say it's *required*? – Ken White Aug 07 '22 at 03:32
  • 2
    You should pay extra attention to 'FreeOnTerminate', as Remy said below. `AThread.FreeOnTerminate := true; AThread.WaitFor;` alone always causes access violation, because `AThread` is destroyed before `WaitFor` returns. And `Destroy` calls `WaitFor`. So, never set `FreeOnTerminate' and try to manage it in any way. – W. Chang Aug 07 '22 at 08:46
  • https://github.com/coderserdar/Documents check the Delphi folder for the Object Pascal Handbook, and the two books (More) Coding in Delphi. – Delphi Coder Aug 07 '22 at 08:58
  • 2
    @KenWhite, saying that types should be prefixes with a T straight after telling the questioner to learn how to create objects correctly implies that it's a requirement. And, for what it's worth, it's a convention that I have strongly avoided since the days of Turbo Pascal when the manual had printed 'tabs' for each initial letter to help you find things, but the tabs were rendered virtually useless as almost everything started with 'T'! – Philip J. Rayment Aug 07 '22 at 12:57

1 Answers1

11

The code in Button1Click() is wrong. You are calling Create() as an instance method on your mySideTask variable, but it is not pointing at a valid object instance. You need to instead call Create() as a constructor on the class type itself.

Try this instead:

procedure TForm1.Button1Click(Sender: TObject);
begin
  mySideTask := myThread.Create(False{True});
  //mySideTask.FreeOnTerminate := True;
  //mySideTask.Start;
end;

Notice I commented out the handling of FreeOnTerminate=True. The reason for that is because that setting is meant for create-and-forget type of threads. The thread will destroy itself after its Execute() method exits. So it is not safe to call WaitFor() or Free() on a thread that could destroy itself at any moment.

If you want to use FreeOnTerminate=True, then the code should look more like this instead:

type
  myThread = class(TThread)
  protected
    procedure Execute; override;
  end;

  TForm1 = class(TForm)
    Button1: TButton;
    Memo1: TMemo;
    procedure Button1Click(Sender: TObject);
    procedure FormClose(Sender: TObject; var Action: TCloseAction);
  private
    { Private declarations }
    mySideTask : myThread;
    procedure SideTaskTerminated(Sender: TObject);
  public
    { Public declarations }
  end;
  
var
  Form1: TForm1;

implementation

{$R *.dfm}

procedure TForm1.Button1Click(Sender: TObject);
begin
  mySideTask := myThread.Create(True);
  mySideTask.FreeOnTerminate := True;
  mySideTask.OnTerminated := SideTaskTerminated;
  mySideTask.Start;
end;

procedure TForm1.SideTaskTerminated(Sender: TObject);
begin
  mySideTask := nil;
end;

procedure TForm1.FormClose(Sender: TObject; var Action: TCloseAction);
begin
  if mySideTask <> nil then
  begin
    mySideTask.FreeOnTerminate := False;
    mySideTask.Terminate;
    mySideTask.WaitFor;
    FreeAndNil(mySideTask);
  end;
end;

{ myThread }

procedure myThread.Execute;
begin
  Synchronize(
    procedure
    begin
        Form1.Memo1.Lines.Add('running my side task')
    end);
end;
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770