Delphi TCPClient read string from TCPServer

Question

I need to write a simple chat program that will be used by some customers. Basically, there are a lot of clients connected to a server and they chat together. The server works:

Here the code if needed:

//CONNECT TO THE SERVER
procedure TFormServer.ButtonStartClick(Sender: TObject);
begin
  if not TCPServer.Active then
    begin
      try
        TCPServer.DefaultPort := 8002;
        TCPServer.Bindings[0].IP := LIP.Text;
        TCPServer.Bindings[0].Port := StrToInt(LPort.Text);
        TCPServer.MaxConnections := 5;
        TCPServer.Active := true;

        Memo1.Lines.Add(TimeNow + 'Server started.');
      except
        on E: Exception do
          Memo1.Lines.Add(sLineBreak + ' ====== INTERNAL ERROR ====== ' +
            sLineBreak + ' > ' + E.Message + sLineBreak);
      end;
    end;
end;

//DISCONNECT
procedure TFormServer.ButtonStopClick(Sender: TObject);
begin
  if TCPServer.Active then
    begin
      TCPServer.Active := false;
      Memo1.Lines.Add(TimeNow + 'Server stopped.');
    end;
end;

//IF CLOSE THE APP DONT FORGET TO CLOSE SERVER!!
procedure TFormServer.FormClose(Sender: TObject; var Action: TCloseAction);
begin
  ButtonStopClick(Self);
end;

procedure TFormServer.FormCreate(Sender: TObject);
begin
  FClients := 0;
end;

//When a client connects I write a log
procedure TFormServer.TCPServerConnect(AContext: TIdContext);
begin
  Inc(FClients);
  TThread.Synchronize(nil, procedure
                           begin
                             LabelCount.Text := 'Connected sockets: ' + FClients.ToString;
                             Memo1.Lines.Add(TimeNow + ' Client connected @ ' + AContext.Binding.IP + ':' + AContext.Binding.Port.ToString);
                           end);
end;

//Same, when a client disconnects I log it
procedure TFormServer.TCPServerDisconnect(AContext: TIdContext);
begin
  Dec(FClients);
  TThread.Synchronize(nil, procedure
                           begin
                             LabelCount.Text := 'Connected sockets: ' + FClients.ToString;
                             Memo1.Lines.Add(TimeNow + ' Client disconnected');
                           end);
end;

//WHAT I DO HERE:
//I receive a message from the client and then I send this message to EVERYONE that is connected here. It is a global chat
procedure TFormServer.TCPServerExecute(AContext: TIdContext);
var
  txt: string;
begin
  txt := AContext.Connection.IOHandler.ReadLn();
  AContext.Connection.IOHandler.WriteLn(txt);
  TThread.Synchronize(nil, procedure
                           begin
                             Memo1.Lines.Add(TimeNow + txt);
                           end);
end;

The sever code is very easy and minimal but it does what I need. This is the client instead:

Here there is the code, very simple:

//CONNECT TO THE SERVER
procedure TFormClient.ConnectClick(Sender: TObject);
begin

  if Length(Username.Text) < 4 then
    begin
      Memo1.Lines.Clear;
      Memo1.Lines.Add('ERROR: Username must contain at least 4 characters');
      Exit;
    end;

  if not TCPClient.Connected then
    begin
      try
        Username.Enabled := false;
        Memo1.Lines.Clear;

        TCPClient.Host := '127.0.0.1';
        TCPClient.Port := 8002;
        TCPClient.ConnectTimeout := 5000;
        TCPClient.Connect;

        Connect.Text := 'Disconnect';
      except
        on E: Exception do
          Memo1.Lines.Add(' ====== ERROR ======' + sLineBreak +
            ' > ' + E.Message + sLineBreak);
      end;
    end
  else
    begin
      TCPClient.Disconnect;
      Username.Enabled := true;
      Connect.Text := 'Connect';
    end;
end;

//IF YOU FORGET TO DISCONNECT WHEN APP IS CLOSED
procedure TFormClient.FormDestroy(Sender: TObject);
begin
  if TCPClient.Connected then
    TCPClient.Disconnect;
end;

//Here I send a string to the server and it's good
procedure TFormClient.SendClick(Sender: TObject);
begin
  if TCPClient.Connected then
    begin
      TCPClient.IOHandler.WriteLn(Username.Text + ': ' + EditMessage.Text);
      EditMessage.Text := '';
    end
  else
    begin
      Memo1.Lines.Add('ERROR: You aren''t connected!');
    end;
end;

//Problems here
procedure TFormClient.Timer1Timer(Sender: TObject);
begin
  Memo1.Lines.Add(TCPClient.IOHandler.ReadLn());
end;

---

The problems start in the last procedure Timer1Timer. I have found that TCPServer uses a thread and this is why I call Synchronize to update the UI. Instead TCPClient does not use a thread and I manually have to check the server. Please see this code:

    procedure TFormServer.TCPServerExecute(AContext: TIdContext);
    var
      txt: string;
    begin
      txt := AContext.Connection.IOHandler.ReadLn();
      AContext.Connection.IOHandler.WriteLn(txt);
      TThread.Synchronize(nil, procedure
                               begin
                                 Memo1.Lines.Add(TimeNow + txt);
                               end);
    end;

As you can see, when the server receives a string he immediatly sends it back to all the clients. I try to get the string here:

procedure TFormClient.Timer1Timer(Sender: TObject);
begin
  Memo1.Lines.Add(TCPClient.IOHandler.ReadLn());
end;

What is wrong? I have seen a similar question here and answers said that I have to use a timer and IOHandler.ReadLn(), which is what I am doing. I think that the problem is here. How to fix?

Also timer has an interval of 200, is it too short?

---

I have read what Remy Lebeau said in the answer and I've produced this simple code:

procedure TFormClient.Timer1Timer(Sender: TObject);
begin
  if not(TCPClient.Connected) then
    Exit;
  if TCPClient.IOHandler.InputBufferIsEmpty then
    Exit;
  Memo1.Lines.Add(TCPClient.IOHandler.InputBufferAsString());
end;

There is a Timer1 component in the form. This works as I expect but it still could lock the UI or not?

Answer 1

The server works

Just an FYI, you don't need the FClients variable at all, especially since you are not really accessing it safely. At the very least, use TInterlocked to access it safely. Or switch to TIdThreadSafeInteger. Though really, the only place you use it is in LabelCount, and you can get the current client count from the TIdTCPServers.Contexts property instead.

This is the client instead:

...

The problems start in the last procedure Timer1Timer.

That is because you are using a UI-based TTimer, and (like most things in Indy) the IOHandler.ReadLn() method blocks until completed. You are calling it within the context of the UI thread, so it blocks the UI message loop until a full line has arrived from the socket.

One way to work around blocking the UI is to place an Indy TIdAntiFreeze component onto your Form. Then the UI will remain responsive while ReadLn() blocks. However, this would be kind of dangerous to use with a TTimer, as you would end up with OnTimer reentry issues that could corrupt the IOHandler's data.

Really, the best solution is to simply not call the IOHandler.ReadLn() in the UI thread at all. Call it in a worker thread instead. Start the thread after you successfully Connect() to the server, and terminate the thread when disconnected. Or even move the Connect() itself into the thread. Either way, you can use Indy's TIdThreadComponent, or write your own T(Id)Thread-derived class.

Instead TCPClient does not use a thread and I manually have to check the server.

Correct, but the way you are doing it is wrong.

If you don't want to use a worker thread (which you should), then at least change your OnTimer event handler so it does not block the UI anymore, like this:

Or:

procedure TFormClient.Timer1Timer(Sender: TObject);
begin
  if TCPClient.IOHandler.InputBufferIsEmpty then
  begin
    TCPClient.IOHandler.CheckForDataOnSource(0);
    TCPClient.IOHandler.CheckForDisconnect(False);
    if TCPClient.IOHandler.InputBufferIsEmpty then Exit;
  end;
  // may still block if the EOL hasn't been received yet...
  Memo1.Lines.Add(TCPClient.IOHandler.ReadLn);
end;

Alternatively:

procedure TFormClient.Timer1Timer(Sender: TObject);
begin
  // Connected() performs a read operation and will buffer
  // any pending bytes that happen to be received...
  if not TCPClient.Connected then Exit;
  while TCPClient.IOHandler.InputBuffer.IndexOf(Byte($0A)) <> -1 do
    Memo1.Lines.Add(TCPClient.IOHandler.ReadLn());
end;

I have seen a similar question here and answers said that I have to use a timer and IOHandler.ReadLn(), which is what I am doing.

Whoever said that is wrong, or you misunderstood what was required. Using a timer in the UI is a possible solution, if used correctly, but it is not a very good solution.

Answer 2

Create a thread for your tcpclient and sync messages back to the UI.