Synchronize Method, can I use it to not main thread?

Question

I rarely use threads and I have question about this class:

unit ExpectingThread;

interface

uses
    System.Classes;

type
    TExpectingThread = class(TThread)
    private
        _timeoutMs: Integer;
        _buff: string;
        _patterns: TArray<string>;
        _result: Integer;
        function Timeouted(startTime: Cardinal): Boolean;
        function ExpectedDetected: Boolean;

    protected
        procedure Execute; override;
    public
        constructor Create(patterns: TArray<string>; buff: string; timeoutMs: Integer);
        //this method is called from other NOT MAIN thread
        procedure BuffUpdate(text: string);
    end;

implementation

uses
    Winapi.Windows,
    System.RegularExpressions;

{ TExpectingThread }

constructor TExpectingThread.Create(patterns: TArray<string>; buff: string; timeoutMs: Integer);
begin
    _patterns := patterns;
    _timeoutMs := timeoutMs;
    _buff := buff;
end;

//this method is called from other NOT MAIN thread
procedure TExpectingThread.BuffUpdate(text: string);
begin
    // lock
    TThread.Synchronize(Self, procedure
        begin
            _buff := _buff + text;
        end);
    // unlock
end;

procedure TExpectingThread.Execute;
var
    startTime: Cardinal;
begin
    inherited;

    startTime := GetTickCount;
    while true do
    begin
        if Timeouted(startTime) then
        begin
            Self.ReturnValue := 0; // timeouted
            Exit;
        end;

        if ExpectedDetected then
        begin
            Self.ReturnValue := 1; // found
            Exit;
        end;
    end;
end;

function TExpectingThread.ExpectedDetected: Boolean;
var
    regex: TRegEx;
    i: Integer;
begin
    // lock
    result := 0;
    for i := 0 to High(_patterns) do
    begin
        regex.Create(_patterns[i]);
        if regex.IsMatch(_buff) then
        begin
            _result := i;
            Exit(true);
        end;
    end;
    // unlock
end;

function TExpectingThread.Timeouted(startTime: Cardinal): Boolean;
var
    currentTime: Cardinal;
begin
    currentTime := GetTickCount;
    result := currentTime - startTime > _timeoutMs;
end;

end.

Thread has to cheacking if any pattern is match to buffer over timeout. But other thread(NOT MAIN) can change buffer by using BuffUpdate method. Did I use Synchronization method correctly?

Answer 1

Synchronize() is specifically designed to work with the main UI thread. You can use it for inter-thread syncing, however ALL threads involved would have to use it. In your example, only the thread(s) that write to _buff are using it, but the thread that reads from _buff is not. So that is a hole in your logic.

That being said, if the main UI thread does not need to touch your data, then Synchronize() is not the best solution to use. You can just wrap the data access with a synchronization object instead, like a TCriticalSection, TMutex, TEvent, TMREWSync, Sytem.TMonitor, etc. For example:

unit ExpectingThread;

interface

uses
  System.Classes, System.SyncObjs;

type
  TExpectingThread = class(TThread)
    private
      _timeoutMs: Integer;
      _buff: string;
      _buffLock: TCriticalSection;
      _buffChanged: Boolean;
      _patterns: TArray<string>;
      _result: Integer;
      function Timeouted(startTime: Cardinal): Boolean;
      function ExpectedDetected: Boolean;
    protected
      procedure Execute; override;
    public
      constructor Create(patterns: TArray<string>; buff: string; timeoutMs: Integer);
      destructor Destroy; override;
      //this method is called from other NOT MAIN thread
      procedure BuffUpdate(text: string);
    end;

implementation

uses
  Winapi.Windows, System.RegularExpressions;

{ TExpectingThread }

constructor TExpectingThread.Create(patterns: TArray<string>; buff: string; timeoutMs: Integer);
begin
  inherited Create(False);
  _buffLock := TCriticalSection.Create;
  _patterns := patterns;
  _timeoutMs := timeoutMs;
  _buff := buff;
  _buffChanged := True;
end;

destructor TExpectingThread.Destroy;
begin
  _buffLock.Free;
  inherited;
end;

//this method is called from other NOT MAIN thread
procedure TExpectingThread.BuffUpdate(text: string);
begin
  _buffLock.Enter;
  try
    _buff := _buff + text;
    _buffChanged := True;
  finally
    _buffLock.Leave;
  end;
end;

procedure TExpectingThread.Execute;
var
  startTime: DWORD;
begin
  startTime := GetTickCount;
  while not Terminated do
  begin
    if Timeouted(startTime) then
    begin
      Self.ReturnValue := 0; // timeouted
      Exit;
    end;
    if ExpectedDetected then
    begin
      Self.ReturnValue := 1; // found
      Exit;
    end;
  end;
end;

function TExpectingThread.ExpectedDetected: Boolean;
var
  i: Integer;
  buff: string;
begin
  Result := False;
  _buffLock.Enter;
  try
    If not _buffChanged then Exit;
    buff := _buff;
    UniqueStr(buff);
    _buffChanged := False;
  finally
    _buffLock.Leave;
  end;
  for i := Low(_patterns) to High(_patterns) do
  begin
    if TRegEx.IsMatch(buff, _patterns[i]) then
    begin
      _result := i;
      Exit(True);
    end;
  end;
end;

function TExpectingThread.Timeouted(startTime: Cardinal): Boolean;
var
  currentTime: DWORD;
begin
  currentTime := GetTickCount;
  result := currentTime - startTime > _timeoutMs;
end;

end.