Using TCP in a TThread crashes with form open lazarus / 38-Networking and Web Programming ---------------------------------------------------------------------- geraldholdsworth · 2023-09-06 15:47 Still on my voyage of discovery here, 30-odd years after first starting out with Delphi. This time, I've started to use Threads for the first time. I'm also trying to write a logger application where strings are sent from another machine, via TCP, and these are logged in a file as well as a TMemo. So, this started out well - I used the built in networking socket units instead of something like Synapse because it meant I didn't need to install anything, and it also gave me access to the remote mahcine's IP address. First version worked OK - although when the TInetServer was started, that hung the program. I then added in the Idle time and an OnIdle procedure to get control back. Worked fine. However, I thought I'd try sticking this into a Thread, so it runs concurrently, allowing it to sit and wait as it pleases. Again, this worked fine until I added a new form. Now, if the logging is started with the form open, it crashes. However, if the logging is started with the form not open it works fine, despite the same procedure being run on both occasions. Confused? I am! Basically, it crashes after exiting btnStartClick procedure, with the form open. There is an autostart functionality, which I've disabled for now...made no difference to when it wasn't disabled. I'm using Lazarus 2.2.6 on macOS Ventura, M2 chip. Oddly, the FPC is ARM while Lazarus is x86_64 (that's just how I downloaded it, and how it is made available). BTW, this is something I'm writing for my company, hence the company name being splashed all over it. The evenutal idea is to have this running on a RaspberryPi, hidden away in a cupboard with the rest of the kit, and used to diagnose issues. It may even get run on Windows...who knows? ---------------------------------------------------------------------- zeljko · 2023-09-06 16:03 Your problem is using of MainForm in thread eg inside procedure TTCPThread.WriteToLog(Host, Output: String); without Synchronize().  That's a big no no. ---------------------------------------------------------------------- geraldholdsworth · 2023-09-06 16:07 Quote from: zeljko on September 06, 2023, 06:03:36 pmYour problem is using of MainForm in thread eg inside procedure TTCPThread.WriteToLog(Host, Output: String); without Synchronize().  That's a big no no. Ahh..thank you. How does one use Synchronize, or even, how would I write to the TMemo on the form from inside the thread? ---------------------------------------------------------------------- geraldholdsworth · 2023-09-06 16:39 I've changed it, and this works, thus: Added a new property in the TTCPThread Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i FLog        : TStringList;and under published Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i property Log        : TStringList read FLog write FLog;Added lines in the class constructor and destructor to create and free this TStringList. Changed the WriteToLog, in the class, to add the string to this list instead of the Memo on MainForm. Then, in MainForm, rewritten the RefreshLogTimer procedure: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i procedure TMainForm.RefreshLogTimer(Sender: TObject);begin while FServer.Log.Count>0 do begin  WriteToLog(FServer.Log[0]);  FServer.Log.Delete(0); end;end;Seems to do the trick. Again, thank you. Still on a learning curve ---------------------------------------------------------------------- alpine · 2023-09-06 17:46 Sorry for the intervention, but now your FLog: TStringList is not mutually excluded. You must at least enclose access to it in a 'critical section'. In WriteToLog and in RefreshLogTimer as well. ---------------------------------------------------------------------- geraldholdsworth · 2023-09-06 17:54 Quote from: alpine on September 06, 2023, 07:46:56 pmSorry for the intervention, but now your FLog: TStringList is not mutually excluded. You must at least enclose access to it in a 'critical section'. In WriteToLog and in RefreshLogTimer as well. No need to apologise - it's how we learn from each other. Can you explain about a 'critical section', and how I enclose the log inside it? Or should I create an 'OnLog' event handler for the class and pick this up outside? ---------------------------------------------------------------------- geraldholdsworth · 2023-09-06 18:18 I've followed this guide and created an event to update the log. ---------------------------------------------------------------------- alpine · 2023-09-06 18:21 Quote from: geraldholdsworth on September 06, 2023, 07:54:27 pmNo need to apologise - it's how we learn from each other. Can you explain about a 'critical section', and how I enclose the log inside it? Or should I create an 'OnLog' event handler for the class and pick this up outside? I found this in the Wiki, don't know how good is it: https://wiki.free pascal.org/Multithreaded_Application_Tutorial#Critical_sections The MT is a complicated subject, but the long story short - two different threads can't modify one resource without 'mutual exclusion', i.e. they must modify it one at a time. Otherwise, when RefreshLogTimer is executing it may be interrupted in the middle of e.g. FServer.Log.Delete(0) and then WriteToLog to execute FLog.Insert(0). This will result in complete mess, since the TStringList is not "thread safe". One solution is to declare and create a FCS: TCriticalSection into the TTCPThread and into the WriteToLog to enclose the access to FLog with Acquire/Release : Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i   FCS.Acquire;  try    FLog.Add(Msg);  finally    FCS.Release;  endAnd then to provide a method, again with access to FLog enclosed in Acquire/Release : Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i function TTCPThread.Fetch(out Msg: String): Boolean;begin  FCS.Acquire;  try  if FLog.Count > 0 then    begin      Msg := FLog[0];      FLog.Delete(0);      Result := True;    end    else      Result := False;  finally    FCS.Release;  end;end;  Then you can use Fetch to retrieve safely the log lines. Of course, the code is just to give you the idea. ---------------------------------------------------------------------- geraldholdsworth · 2023-09-07 07:09 Thank you alpine, very helpful. I did read that document, but your explaination was easier to understand. Before you sent that, I changed the method and went down the event route, so the form gets notified as opposed to having to poll. Are there any advantages to either method? What is the preferred method? Or is it just a case of 6 of one and half a dozen of the other? ---------------------------------------------------------------------- alpine · 2023-09-07 08:56 If by "event route" you mean calling the: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i Synchronize(@Showstatus)actually you're using the term "event" in the context of a GUI event, there is a TEvent class into the syncobjs unit which is a MT synchronization/propagation mechanism.  As zeljko wrote earlier Quote from: zeljko on September 06, 2023, 06:03:36 pmYour problem is using of MainForm in thread eg inside procedure TTCPThread.WriteToLog(Host, Output: String); without Synchronize().  That's a big no no. He actually meant that you can't execute any GUI stuff outside the main GUI thread. If you want to do so, TThread.Synchronize is the simplest way. The Syncronize argument (which is a procedure of object, i.e. it have a Self pointer attached) gets inserted in a queue and the caller thread gets blocked until the main GUI thread executes it. You don't have to "mutual exclude" the shared resource (e.g. your TStringList), since it is guaranteed that your thread is blocked during the execution of the procedure and won't interfere. Another similar mechanism is QueueAsyncCall, where the procedure is again inserted into the GUI thread queue, but the calling thread continues its execution. In that case the shared resource must be mutually excluded and in addition - its existence must be guaranteed by the time of the (future) execution by the main thread - i.e. it shouldn't be freed until then. That usually means a proper clean-up procedure at exit. Polling with a timer is similar to the latter one, the TTimer.OnTimer gets executed in response of a timer message, which is handled into the GUI thread. The above considerations are also valid - mutual exclusion, etc. The difference is that visualization is on regular intervals. Usually, the shared resource (FLog: TStringList in your case) is a specially crafted "thread safe" descendant of a container. The TThread.Syncronize is the simplest way, but is a way slooow, the pacer is the GUI thread The QueueAsyncCall is a way better, but it comes with a complicationsThe timer polling is a bit simpler than QueueAsyncCall (you can just disable it on exit) but it may introduce visualization delays ---------------------------------------------------------------------- marcov · 2023-09-07 09:55 QueueAsyncCall is an early lazarus specific iteration of postmessage() that Delphi (D2006) and FPC(3.2?) later adapted as TThread.Queue ---------------------------------------------------------------------- Thaddy · 2023-09-07 11:28 Synchronize() on it self loses any thread benefits, use it only to display or log the values from a thread. This is a very common and well documented mistake. In essence, let the thread perform its task and then and only then call synchronize to communicate with the main thread. Synchronize alone means the thread has to wait for synchronization with the main thread, so you should minimize the code that relies on synchronization. (one liners, basically) So never ever run all code from a thread under synchronize() because then your program is not properly multi-threaded... Use it with care and only use it for communication of a result of a thread. Aside: thread programming looks easy but it is NOT at all. ---------------------------------------------------------------------- geraldholdsworth · 2023-09-07 12:37 Quote from: Thaddy on September 07, 2023, 01:28:45 pmSynchronize() on it self loses any thread benefits, use it only to display or log the values from a thread. This is a very common and well documented mistake. A-ha...I understand now. So Synchronize, although works, is not as good as using critical sections. Thank you all for your helpful explainations. I'll go with alpine's suggestion of critical sections. I don't want to be blocking the thread. Quote from: Thaddy on September 07, 2023, 01:28:45 pmAside: thread programming looks easy but it is NOT at all. Maybe that's why I've put off doing threads for 30-odd years! ---------------------------------------------------------------------- alpine · 2023-09-07 16:01 Quote from: geraldholdsworth on September 07, 2023, 02:37:53 pmQuote from: Thaddy on September 07, 2023, 01:28:45 pmSynchronize() on it self loses any thread benefits, use it only to display or log the values from a thread. This is a very common and well documented mistake. A-ha...I understand now. So Synchronize, although works, is not as good as using critical sections. Thank you all for your helpful explainations. I'll go with alpine's suggestion of critical sections. I don't want to be blocking the thread. Quote from: Thaddy on September 07, 2023, 01:28:45 pmAside: thread programming looks easy but it is NOT at all. Maybe that's why I've put off doing threads for 30-odd years! Here is a boilerplate for you: Code: Pascal  [Select][+][-]window.onload = function(){var x1 = document.getElementById("main_content_section"); if (x1) { var x = document.getElementsByClassName("geshi");for (var i = 0; i unit Unit1; {$mode objfpc}{$H+} interface uses  Classes, SysUtils, Forms, Controls, Graphics, Dialogs, StdCtrls,  syncobjs; type  TOnLogText = procedure(Sender: TObject; Line: String) of object;   { TTCPThread }   TTCPThread = class(TThread)  private    FOnLogText: TOnLogText;  public    procedure Execute; override;    property OnLogText: TOnLogText read FOnLogText write FOnLogText;  end;   { TMainForm }   TMainForm = class(TForm)    Memo1: TMemo;    procedure FormCreate(Sender: TObject);    procedure FormDestroy(Sender: TObject);  private    FServer: TTCPThread;    FCritical: TCriticalSection;    FLog: TStringList;  public    procedure OnLogText(Sender: TObject; Line: String);    procedure OnLogTextAsync(Data: PtrInt);  end; var  MainForm: TMainForm; implementation {$R *.lfm} { TTCPThread } procedure TTCPThread.Execute;var  Msg: String;begin  while not Terminated do  begin    {Receive the log message into Msg from somewhere...}    Sleep(1000); Msg := 'Message ' + GetTickCount64.ToString;     if Assigned(FOnLogText) then      FOnLogText(Self, Msg);  end;end; { TMainForm } procedure TMainForm.FormCreate(Sender: TObject);begin  FCritical := TCriticalSection.Create;  FLog := TStringList.Create;  FServer := TTCPThread.Create(True);  FServer.OnLogText := @OnLogText;  {Other FServer initializations}  FServer.Start;end; procedure TMainForm.FormDestroy(Sender: TObject);begin  FServer.Terminate;  FServer.WaitFor;  FServer.Free;  Application.RemoveAsyncCalls(Self);  FLog.Free;  FCritical.Free;end; // Called into the context of the TTCPThreadprocedure TMainForm.OnLogText(Sender: TObject; Line: String);var  C: Integer;begin  FCritical.Acquire;  try    // Modify FLog in a mutually exclusive manner    C := FLog.Add(Line);  finally  FCritical.Release;  end;  // Queue only one call on multiple messages  if C = 0 then    Application.QueueAsyncCall(@OnLogTextAsync, 0);end; // Called into the context of the main threadprocedure TMainForm.OnLogTextAsync(Data: PtrInt);var  L: array of String;begin  FCritical.Acquire;  try    // Access/modify FLog in a mutually exclusive manner    L := FLog.ToStringArray; // All lines at once    FLog.Clear;  finally    FCritical.Release;  end;  // Do the GUI stuff outside the critical section  Memo1.Lines.AddStrings(L);end; end.  I must note that I've never used TInetServer, but it seems to me that as it is written into the initial post, only one connection will be possible. Is that enough? ---------------------------------------------------------------------- geraldholdsworth · 2023-09-07 19:51 That looks interesting. One thing I notice though - I couldn't get TCriticalSection.Acquire, etc. to work - Lazarus wouldn't recognise it. So I added LCLIntf and LCLType which exposed IntializeCriticalSection, EnterCriticalSection and LeaveCriticalSection. But then looking at the SyncObjs unit, this just uses these commands anyway inside the .Acquire, etc. Quote from: alpine on September 07, 2023, 06:01:00 pmI must note that I've never used TInetServer, but it seems to me that as it is written into the initial post, only one connection will be possible. Is that enough? Yep, one connection is enough. If I need more, I can create more TTCPServer objects...but I'll cross that bridge when I come to it. ---------------------------------------------------------------------- MarkMLl · 2023-09-08 07:29 Quote from: geraldholdsworth on September 07, 2023, 02:37:53 pmA- ha...I understand now. So Synchronize, although works, is not as good as using critical sections. Different use cases. Synchronize() works between threads, but critical sections can also prevent a single thread reentering code. I know that sounds weird, but if you call an APM inside the shim handling a dequeued synchronise the main thread can dequeue another and reenter. BTDT, nasty to find. MarkMLl ---------------------------------------------------------------------- MarkMLl · 2023-09-08 07:38 Quote from: geraldholdsworth on September 06, 2023, 05:47:32 pmSo, this started out well - I used the built in networking socket units instead of something like Synapse because it meant I didn't need to install anything, and it also gave me access to the remote mahcine's IP address. First version worked OK - although when the TInetServer was started, that hung the program. I then added in the Idle time and an OnIdle procedure to get control back. Worked fine. If that's the version of LNet bundled with FPC, I'd warn that my understanding is that it's in there to support installation and is not really intended for general use. I've added stuff to it in the past (slight improvement to Telnet handling), but the master implementation is distinct and on Github (?). However, I hit some very nasty problems relating to client disconnection followed by an immediate reconnection attempt, and found the LNet code extremely difficult to work through: I think it predates threads hence, now that they are universally implemented, is effectively obsolete. In the end I wrote my own Telnet server and client code, and took the time to expunge LNet from anywhere I'd been using it (e.g. to provide a diagnostic login to recover a program's activity log). If using a standard protocol, use Synapse etc.: apart from anything else that's your best route to getting e.g. SSL support. If implementing a non-standard protocol, familiarise yourself with the underlying Berkeley Sockets API provided by most OSes. MarkMLl ---------------------------------------------------------------------- geraldholdsworth · 2023-09-08 08:22 Quote from: MarkMLl on September 08, 2023, 09:38:32 amIf using a standard protocol, use Synapse etc.: apart from anything else that's your best route to getting e.g. SSL support.I don't need SSL, but I have looked at Synapse before now. Can't quite remember why I discounted it at the time - I did use another Synapse module when I wrote a program to send and receive over RS232, which worked well. And I didn't need to install anything, just 'use' the appropriate module(s). I'll probably move it all over to Synapse, eventually - I've been looking at the 'echo' demo supplied, and working out what it's doing. Looks fairly simple enough.