0

I am writing a custom class that should have the ability to connect to a TMemo FireMonkey component on the form in order to log info to it. The class is defined as :

TBlokData = class
private
  [weak] FLogMemo: TMemo;
  procedure Log(s : string);
public
  constructor Create(ConnStr: string);
  property LogMemo : TMemo read FLogMemo write FLogMemo;
  destructor Destroy; override;
end;

and the implementation of the Log method is :

procedure TBlokData.Log(s : string);
begin
    if Assigned(FLogMemo) then
        FLogMemo.Lines.Add(TimeToStr(Now) + ': ' + s);
end;

I am concerned if I create the class object in a thread and populate the LogMemo property with, say, the Memo1 component on the FireMonkey form, that my class will no longer be thread-safe because I will manipulate a component on the form from a thread when calling the Log method.

Is this a valid concern? If so, how can I make it thread-safe while maintaining the class' usability outside a threading environment as well?

Edrean Ernst
  • 370
  • 1
  • 14

2 Answers2

1

You are right to be concerned. Your code is not threadsafe. Use TThread.Synchronize or TThread.Queue to ensure that any manipulation of the UI control are performed on the main UI thread.

As a broad rule, any manipulation of a UI component must be performed on the UI thread.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
1

Create a method LogThreadSafe:

  TThread.Synchronize(nil, procedure 
    begin
      Log(s);
    end); 
David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
Wosi
  • 41,986
  • 17
  • 75
  • 82
  • 1
    Doesn't Synchronize already check the thread ID? No point checking twice. – David Heffernan Aug 26 '15 at 17:00
  • http://docwiki.embarcadero.com/Libraries/XE8/en/System.Classes.TThread.Synchronize : "**Warning:** Do not call Synchronize from within the main thread. This can cause an infinite loop." – Wosi Aug 26 '15 at 17:06
  • 1
    That documentation is wrong. `TThread.Synchronize` checks the thread ID, and if we are on the main thread, it just calls the method directly and returns. It's a little pointless to pass `TThread.CurrentThread`. Better to pass `nil`. – David Heffernan Aug 26 '15 at 17:12
  • 4
    Thanks for pointing this out. You're right. Sometimes I have the feeling Embarcadero has two kinds of documentation: Wrong and missing – Wosi Aug 26 '15 at 17:16
  • http://stackoverflow.com/questions/29919183/can-i-move-delphi-tthread-synchronize-locally-to-a-vcl-form-to-be-called-from – David Heffernan Aug 26 '15 at 17:17
  • Thanks guys! I was worried about the implication of using `TThread.Synchronize` in the main thread. I guess the EMBT developers implemented the check in Synchronize at some stage and forgot to tell everyone about it ;-). – Edrean Ernst Aug 28 '15 at 07:56