I am working with streaming packet strings through Indy sockets, and on the client side, I have a thread which reads incoming data from the TIdTCPClient
and continuously appending this data to the end of a single string buffer. I have another thread which is continuously reading this buffer from the beginning, copying (and deleting) data as necessary (one complete packet at a time).
I know that in any case two threads accessing the same variable can be dangerous. But does this apply to strings too? Or just objects? Can I feel safe with this reading/writing of the same string from two different threads? If not, then what should I do to protect this string? This is a plain string called FBuffer
.
I'm appending data to the end like so:
procedure TListenThread.CheckForData;
begin
if FClientSocket.Connected then begin
FClientSocket.IOHandler.CheckForDataOnSource(5000);
if not FClientSocket.IOHandler.InputBufferIsEmpty then
FBuffer:= FBuffer + FClientSocket.IOHandler.InputBufferAsString;
end;
end;
And the other thread is reading it like so:
procedeure TPacketThread.CheckForPacket;
var
P: Integer; //Deliminator position
T: String; //Temp copying string
Z: Integer; //Expected packet size
begin
P:= Pos('#', FBuffer);
if P > 0 then begin //Is the deliminator found?
T:= Copy(FBuffer, 1, P-1); //Copy up to deliminator...
Z:= StrToIntDef(T, 0); //Convert packet size to integer...
if Z > 0 then begin
//Is there a full packet waiting in buffer?
if Length(FBuffer) >= Z then begin
//First, delete size definition and deliminator...
Delete(FBuffer, 1, P);
//Now grab the rest of it up to the packet size...
T:= Copy(FBuffer, 1, Z);
//Delete what we just copied...
Delete(FBuffer, 1, Z);
//Finally, pass this packet string for further processing...
ProcessPacket(T);
end;
end;
end;
end;
The code is a simplified version of my code just to demonstrate all the work I need to do with FBuffer
.
Yes, you must protect the string buffer from concurrent access. Indy has a TIdThreadSafeString
class you can use for that purpose, eg:
FBuffer: TIdThreadSafeString;
// make sure to Create() and Free() as needed..
.
procedure TListenThread.CheckForData;
begin
if FClientSocket.Connected then begin
FClientSocket.IOHandler.CheckForDataOnSource(5000);
if not FClientSocket.IOHandler.InputBufferIsEmpty then
FBuffer.Append(FClientSocket.IOHandler.InputBufferAsString);
end;
end;
.
procedure TPacketThread.CheckForPacket;
var
P: Integer; //Deliminator position
T: String; //Temp copying string
Z: Integer; //Expected packet size
begin
FBuffer.Lock;
try
P:= Pos('#', FBuffer.Value);
if P > 0 then begin //Is the deliminator found?
T := Copy(FBuffer.Value, 1, P-1); //Copy up to deliminator...
Z := StrToIntDef(T, 0); //Convert packet size to integer...
if Z > 0 then begin
//Is there a full packet waiting in buffer?
if Length(FBuffer.Value) >= Z then begin
//First, delete size definition and deliminator...
FBuffer.Value := Copy(FBuffer.Value, P+1, MaxInt);
//Now grab the rest of it up to the packet size...
T := Copy(FBuffer.Value, 1, Z);
//Delete what we just copied...
FBuffer.Value := Copy(FBuffer.Value, Z+1, MaxInt);
//Finally, pass this packet string for further processing...
ProcessPacket(T);
end;
end;
end;
finally
FBuffer.Unlock;
end;
end;
With that said, given what you have shown about the packet formatting, I would take a different tactic instead:
FBuffer: TIdThreadSafeStringList;
// make sure to Create() and Free() as needed..
.
procedure TListenThread.CheckForData;
var
T: String; //Temp copying string
Z: Integer; //Expected packet size
begin
if FClientSocket.Connected then begin
if FClientSocket.IOHandler.InputBufferIsEmpty then begin
FClientSocket.IOHandler.CheckForDataOnSource(5000);
if FClientSocket.IOHandler.InputBufferIsEmpty then Exit;
end;
// data is available, keep reading as long as packets are present...
repeat
T := FClientSocket.IOHandler.ReadLn('#');
Z := StrToIntDef(T, 0);
if Z > 0 then begin
T := FClientSocket.IOHandler.ReadString(Z);
FBuffer.Add(T);
end;
until FClientSocket.IOHandler.InputBufferIsEmpty;
end;
end;
.
procedure TPacketThread.CheckForPacket;
var
L: TStringList;
T: String;
begin
L := FBuffer.Lock;
try
if L.Count = 0 then Exit;
T := L[0];
L.Delete(0);
finally
FBuffer.Unlock;
end;
ProcessPacket(T);
end;
Yes, you must protect the strings when are accessed from multiples threads, you can do that using crtical sections. Take a look to the EnterCriticalSection, LeaveCriticalSection , InitializeCriticalSection and DeleteCriticalSection functions.