Delphi: Why Socket.ReadString(10) sometimes don't get the right data

Question

Server:

procedure TForm1.IdTCPServer1Execute(AContext: TIdContext);
var
  len : integer;
  rest_packet : string;
  packet_length : string;
begin

        try
          // length 10, it will be the rest packet length: 0000000545
          // So the bug is in ReadBytes(10) , some times it do not receive the length 0000000015 but receive a mix of data+length, something like: "00015Hello" or "loWorldApp" etc.
          packet_length := AContext.Connection.Socket.ReadString(10);
        except
          on E:Exception do begin Exit; end;
        end;

        // Convert string to int: 0000000545 , to 545
        try
          len := StrToInt(packet_length);
        except
          on E:Exception do begin Exit; end;
        end;


        // get reset of data, size: 545
        try
          rest_packet := AContext.Connection.Socket.ReadString(len);
        except
          on E:Exception do begin Exit; end;
        end;

Client:

Client is sending data in multithreading to server, about 13 clients is sending data to server thrue one Socket connection. Clients are sending data like this:

    EnterCriticalSection(CS);
    // Memo1.Lines.Add(packet); // i can see here that all works fine.
    Ftunnel.Socket.Write(packet);
    LeaveCriticalSection(CS);

CS - is global, and created like this:

initialization
  InitializeCriticalSection(CS);
finalization
  DeleteCriticalSection(CS);

Structure of the packet is:

LengthDATA

example:

0000000015HelloWorldApple

So the bug is in ReadBytes(10) , some times it do not receive the length 0000000015 but receive a mix of data+length, something like: "00015Hello" or "loWorldApp" etc.

Why this is happening, what I'm doing wrong ?

ReadTimeout is default -1

Here how is look Client code:

procedure ss_thread.Execute;
var ss : TIdTCPClient;
    unix_time : integer;
    data : TIdBytes;
    packet, s : string;
begin
    // connecting to website to get a data
    ss := TIdTCPClient.Create(nil);
    try
      with TIdTcpClient(ss) do
      begin
        Host := '127.0.0.1';
        Port := 80;
        Connect;
      end;
    except
      on E:Exception do
      begin
        ss.Disconnect;
        exit;
      end;
    end;


    // writing a data from server to connected web site server
    try
      ss.Socket.Write(Fff_data);
    except
      on E:Exception do begin end;
    end;


    unix_time := DateTimeToUnix(NOW);


    // getting data from web site server, all looks thread safe?
    while True do
    begin
      ss.Socket.CheckForDataOnSource(5);
      if not ss.Socket.InputBufferIsEmpty then
      begin
        SetLength(data, 0);
        ss.Socket.InputBuffer.ExtractToBytes(data);
        s := TIdEncoderMIME.EncodeBytes(data);
        packet := make_my_length(s) + s;
        ss.Socket.InputBuffer.Clear;
        unix_time := DateTimeToUnix(NOW);
        try
            // sending data from web site, to server
            EnterCriticalSection(CS);
            tunnel.Socket.Write(packet);
            LeaveCriticalSection(CS);
        except
          on E:Exception do begin end;
        end;
      end;

      // disconnecting if no more data for 120sec
      if (DateTimeToUnix(NOW) - unix_time) > 120 then
      begin
        ss.Disconnect;
        break;
      end;

      // killing thread if web site server disconnected us
      if not ss.Connected then
      begin
        break;
      end;

      // disconnecting from web site server and killing thread if tunnel died
      if not tunnel.Connected then
      begin
        ss.Disconnect;
        break;
      end;


    end;

// terminate thread
Terminate;
end;

make my length code, works good:

function make_my_length(s:string):string;
var len, i : integer;
    res : string;
begin
  if s='' then
  begin
    Result := '';
    Exit;
  end;
  res := '';
  len := Length(s);
  if len < 10 then
    for i:=1 to (10 - len) do
    begin
      res := res + '0';
    end;
  Result := res + s;
end;

Delphi 2010, Indy 10, Win7

Answer 1

The symptom you have described would be consistent with your client threads NOT sharing a single CS correctly when accessing Ftunnel, so that multiple writes could continue overlapping each other, which is what the CS is trying to avoid. Are you creating the CS in the same unit as your CheckForData_from_ss() method? Did you verify that each thread is indeed accessing the same CS and not different CS's? Where is CheckForData_from_ss() actually be called from? Accessing a TMemo (or any other UI control, for that matter) from a worker thread directly, like your code suggests you are, is dangerous and can cause all kinds of side effects if memory is getting corrupted. You need to synchronize with the main thread, such as with Indy's TIdSync or TIdNotify class, when accessing the UI.

Another possibility would be if the receiving server is not reading the tunneled data correctly on its end. For instance, if the server missed some bytes at one point, then all subsequent reads would be at the wrong frame offsets. If you are going to frame your tunneled data, then you should add identifying markers to the frames so you will know if the server is receiving valid frames before you try to decode them, and can recover if the markers get out of sync.

EDIT: Try this in your client thread:

var
  CS: TCriticalSection;

procedure ss_thread.Execute; 
var
  ss : TIdTCPClient; 
  data : TIdBytes; 
  packet, s : string; 
begin 
  // connecting to website to get a data 
  ss := TIdTCPClient.Create(nil); 
  try
    ss.Host := '127.0.0.1'; 
    ss.Port := 80; 
    ss.ReadTimeout := 120000;
    ss.Connect; 

    // writing a data from server to connected web site server 
    ss.IOHandler.Write(Fff_data); 

    // getting data from web site server
    while not Terminated do
    begin
      SetLength(data, 0); 
      ss.IOHandler.ReadBytes(data, -1, False);
      s := TIdEncoderMIME.EncodeBytes(data); 
      packet := make_my_length(s) + s; 
      // sending data from web site, to server 
      CS.Enter; 
      try
        tunnel.IOHandler.Write(packet); 
      finally
        CS.Leave; 
      end;
    end; 
  finally
    ss.Free;
  end;
end; 

initialization
  CS := TCriticalSection.Create;
finalization
  CS.Free;