HwTrap
HwTrap

Reputation: 303

Access function result inside thread

I'm working with Threads (again) and this is always pain in the... I have one function inside my thread class, and it's private. The function, returns one boolean result, to check if some POP3 server is valid or no. The checking is working, I tested it, and looks like working at least, but I'm having a problem to access the result when tryin it inside Thread.Execute procedure.. I'll explain more, let's go to the codes... Here is the declaration:

type
  MyThread = class(TThread)
  public
    constructor Create(HostLine: string);
  protected
    procedure Execute; override;
    procedure MainControl(Sender: TObject);
  private
    Host: string;
    function CheckPOPHost: boolean; //this is the problematic function
  end;

So the function is something like this:

function MyThread.CheckPOPHost: boolean;
var
  MySocket: TClientSocket;
  SockStream: TWinSocketStream;
  Buffer: array[0..1023] of Char;
  ReceivedText: string;
begin
  Result:= false;
  FillChar(Buffer, SizeOf(Buffer), #0);
  MySocket:= TClientSocket.Create(Nil);
  MySocket.Port:= 110;
  MySocket.ClientType:= ctBlocking;
  MySocket.Host:= Host;
  MySocket.Active:= true;
  if (MySocket.Socket.Connected = true) then
    begin
      SockStream := TWinSocketStream.Create(MySocket.Socket, 1000);
      SockStream.WaitForData(10000);
      while (SockStream.Read(Buffer, SizeOf(Buffer)) <> 0) do
        ReceivedText:= ReceivedText + Buffer;
      if Length(ReceivedText) > 0 then
        ReceivedText:= PAnsiChar(ReceivedText);
      if AnsiStartsStr('+', ReceivedText) then
        Result:= true;
    end;
  SockStream.Free;
  MySocket.Free;
end;

So, you don't need to read if don't want to. But I'm connecting into some remote host, and receiving it's text, if the text received begins with '+' symbol (default for POP3 servers), I return true... But when I try to do this inside Thread.Execute:

 if CheckPOPHost = true then
    begin
      Form1.Memo1.Lines.Append('Valid HOST:: '+Host);

Just don't work. I think is good to remember, that if inside the function, instead of doing:

  if AnsiStartsStr('+', ReceivedText) then
    Result:= true;

I do:

  if AnsiStartsStr('+', ReceivedText) then
    Form1.Memo1.Lines.Append('Valid HOST:: '+Host);

It works normally... What's happening?!

Edit:: I'm getting error on 'if CheckPOPHost = true then' line. For some unknown reason it's giving Access Violation error.

Upvotes: 0

Views: 1136

Answers (2)

David Heffernan
David Heffernan

Reputation: 612954

You are calling

SockStream.Free

even if you don't enter the block that creates SockStream. And that means your code can call Free on an un-initialized variable. That is the only reason that I can see for the access violation. You need to move the Free inside the if block.

As an aside, always use try/finally when creating objects:

SockStream := TWinSocketStream.Create (MySocket.Socket, 1000);
try
  ....
finally
  SockStream.Free;
end;

Had you written it like this, the compiler would not have let you put the Free in the wrong block.

You should do the same with MySocket.

And don't access you GUI from the thread, but I think others have made that point clearly enough!

Upvotes: 5

LU RD
LU RD

Reputation: 34899

You are calling non-thread-safe VCL methods from a thread.

Form1.Memo1.Lines.Append('Valid HOST:: '+Host);

Never do that.

Wrap the call in a Synchronize( YourThread.SendString); method in the thread.

Example:

MyThread.SendString;
begin
  Form1.Memo1.Lines.Append('Valid HOST:: '+Host);
end;

And somewhere in your thread execute:

if CheckPOPHost = true then
  begin
    Synchronize(SendString);

Update 2:

From your comments about freeing objects in CheckPOPHost: Enclose try..finally blocks around those objects and an additional try..except block inside your CheckPopHost.

Update

A discussion about the thread wizard in the comments arised. This is what you get if you create a thread unit following the wizard interface:

unit Unit21;

interface

uses
  System.Classes;

type
  TMYTHREADTEST = class(TThread)
  private
    { Private declarations }
  protected
    procedure Execute; override;
  end;

implementation

{
  Important: Methods and properties of objects in visual components can only be
  used in a method called using Synchronize, for example,

      Synchronize(UpdateCaption);

  and UpdateCaption could look like,

    procedure TMYTHREADTEST.UpdateCaption;
    begin
      Form1.Caption := 'Updated in a thread';
    end;

    or

    Synchronize( 
      procedure
      begin
        Form1.Caption := 'Updated in thread via an anonymous method' 
      end
      )
    );

  where an anonymous method is passed.

  Similarly, the developer can call the Queue method with similar parameters as 
  above, instead passing another TThread class as the first parameter, putting
  the calling thread in a queue with the other thread.

}

{ TMYTHREADTEST }

procedure TMYTHREADTEST.Execute;
begin
  { Place thread code here }
end;

end.

Upvotes: 6

Related Questions