Simple multithreading Delphi

2
votes

I am still quite new to threading. I want to create a procedure which tests for a valid internet connection while the main thread creates the necessary forms. The code snippet stops at the end of the constructor with a 'Cannot call Start on a running or suspended thread' error. And for some reason the main form is closed after this error

constructor TPingThread.Create(IDThread: Integer);
begin
  Self.FID:=IDThread;
  Self.FreeOnTerminate:=true;
end;

destructor TPingThread.Destroy;
begin
  EndThread(FID);
  inherited;
end;

procedure TPingThread.Execute;
var
  iTimeOuts, K: Byte;
  sWebpage: String;
begin
  inherited;
  iTimeOuts:=0;
  FIdPing:=TIdHTTP.Create(nil);
  for k:=1 to 3 do
    begin
      Try
        FIdPing.ConnectTimeout:=2000;
        sWebpage:=FIdPing.Get('http://www.google.co.za')
      Except
        On Exception do inc(iTimeOuts);
      End;
    end;
  if iTimeOuts=3 then MessageDlg('A working internetconnection is needed to reset your password',mtWarning,[mbOK],0);
  if iTimeOuts=0 then FInternetConnection:=false
  else FInternetConnection:=true;
  FreeAndNil(FIdPing);
end;
1
multithreadingdelphi
You forgot to call inherited in your constructor. – TLama
And don't call inherited in Execute. – LU RD
That yields an Incompatible types error – Marnu123
Inherited Create(false); // Run the thread – LU RD
@Marnu123 Incompatible types because your constructor is an overload. You have to call the base constructor - inherited Create(true) to create the thread suspended – J...

1 Answers

4
votes

There are some problems with your code:

  1. You need to call the inherited constructor:

    constructor TPingThread.Create(IDThread: Integer);
begin
  inherited Create(false); // Or true to create a suspended thread
  Self.FID:=IDThread;
  ...

  2. Remove the inherited call in the Execute method, since this is an abstract declaration of TThread. Not an error per se, but should be avoided for clarity.

  3. Use a try/finally after creating FIdPing in the Execute method.

  4. As @mjn says, there is no need to call EndThread(), since the TThread handles this for you.

  5. Calling the VCL MessageDlg() from a thread is not thread safe. You need to synchronize the call or use the Application.MessageBox, a Delphi wrapper to windows MessageBox. Best solution would be to skip the dialog and pass an error message to the main thread, which would need to know this error anyway.