a write an application that has to handle multiple TCP Connections.
Two of them have sort of a telnet protocol, the others are binary.
For each connection I create an "Object" inherited from TComponent, in
which the connection is initialized as follows:
conn := TIdTcpConnection.Create(self);
conn.CreateIOHandler;
conn.Socket.WriteBufferOpen;
conn.Socket.OnStatus := TcpStatus;
The telnet sort connection is a Command+Response system. Because of
problems I had with the IdTelnetClient, I also use the IdTcpConnection
class here (Linefeed is LF+CR for one response, the others CR+LF)
Executing a command works like that:
conn.Socket.Write(strCommand+CR+LF);
A := conn.Socket.ReadLn(CR+LF);
1. Question
-----------
This works for 99% of the commands, and then I get a Timeout in the
ReadLn. And I even get it when I set the ReadTimeout up to 2500ms,
comparing to a TCP Terminal program this can't be the Server's fault,
there the response is always at once received. It seems to get worse
when talking to two Servers, although I call ReadLn sequentially. I put
the periodically execution of the commands in a TThread so the main
application doesn't freeze, but what could it be that I get those
ReadLnTimeOuts so often?
2. Question
-----------
Is my method of connecting/disconnecting right?
Connecting
If (conn.Socket.Opened) then conn.Socket.Close;
Setting Host/Port
conn.Socket.Open;
Writing
conn.Socket.Write(String);
conn.Socket.WriteBufferFlush;
(The protocol is very time critical, I assumed this returns faster than
using no write buffer)
Disconnecting
conn.Socket.Close;
conn.Socket.InputBuffer.Clear;
(Data left in InputBuffer could make the connection look like open)
I experimented with Disconnect; and Connect; and so on but the
Open/Close methods of the Socket seemed to be most direct to me.
3. Question
-----------
For the binary data connections I created a TReadThread =
Class(TThread), that is created when connection is opened, and
terminated/freed when connection is closed.
Sometimes it occurs, that the thread doesn't terminate. I close the
connection as follows:
conn.Socket.Close;
conn.Socket.InputBuffer.Clear;
if readThread <> nil then begin
// Between here..
readThread.Terminate;
readThread.WaitFor;
FreeAndNil(readThread);
// and here the program freezes in CsrNewThread of a System DLL
end;
The thread is as easy as this (without exception handling):
while (Not Terminated) and conn.Socket.Opened do begin
With conn.Socket do begin
CheckForDataOnSource(1000);
If not Opened then break;
I := InputBuffer.Size;
If I>0 then begin
S := ReadString(I);
If Assigned( FOnRead ) then
FonRead(conn, S); // Not Synchronized read event
end;
end;
End;
One time it happened, the tracer showed me the program was in
CheckForDataSource() method. Is it possible that this methode doesn't
return? I read about this method is not thread safe. I don't access
buffer or read functions during the run-time of this thread.
Indy-Version is the Dev-Snapshot from Fulgan.com
Thanks for your help..
Michael Stieler
>
> Executing a command works like that:
> conn.Socket.Write(strCommand+CR+LF);
> A := conn.Socket.ReadLn(CR+LF);
>
It seems that I have finally answered one of my questions:
conn.Socket.Write(command+CR+LF);
conn.Socket.CheckForDataOnSource(1000);
Result := conn.Socket.ReadLn(CR+LF);
This works, so apparently the ReadLn-Procedure doesn't check if data can
be received but perhaps just works from the InputBuffer.
CheckForDataOnSource() calls the protected methods to receive more data
into the buffer.
I hope you'll find solutions to my other problems as well.
M. Stieler
> It seems that I have finally answered one of my questions:
Not really.
> conn.Socket.CheckForDataOnSource(1000);
You do not need to do that. ReadLn() handles that internally for you,
as do all of the other Read...() methods.
> This works, so apparently the ReadLn-Procedure doesn't check
> if data can be received
Yes, it does.
> perhaps just works from the InputBuffer.
ReadBytes() gets its data from the InputBuffer, and all of the other
Read...() methods are implemented to use ReadBytes(). When the
InputBuffer does not hold enough bytes to satisfy a read operation,
ReadBytes() calls ReadFromSource() to poll the socket and put any
pending data into the InputBuffer.
Gambit
> This works for 99% of the commands
I doubt that. You are using write buffering with no threshold
specified. Which means that no data will ever be written to the
socket until either WriteBufferFlush() or WriteBufferClose() are
called. If you forget to call it, the data is not sent to the server.
Worse, if something else calls WriteBufferOpen() internally before you
can flush it, then your data is completely lost.
> I get a Timeout in the ReadLn.
Because you are using write buffering, it is likely that not all of
your command data is actually being sent to the server, so the
ReadLn() times out because the server is still waiting for you to
finish sending a full command and thus is not sending back a response
at all.
Get rid of the write buffering. You are not using it correctly
anyway, and it is not gaining you anything useful.
> And I even get it when I set the ReadTimeout up to 2500ms,
> comparing to a TCP Terminal program this can't be the Server's
> fault, there the response is always at once received.
Terminal does not use write buffering, so there is no delay in data
being sent to the server, and thus no delay in its response.
> Is my method of connecting/disconnecting right?
No. You should not be creating a TIdTCPConnection directly in the
first place. You should be using a TIdTCPClient instead, ie:
conn := TIdTCPClient.Create(self);
conn.OnStatus := TcpStatus;
...
if conn.Connected then conn.Disconnect;
Set Host/Port
conn.Connect;
...
conn.IOHandler.Write(String);
...
conn.Disconnect;
conn.IOHandler.InputBuffer.Clear;
> The protocol is very time critical, I assumed this returns faster
> than using no write buffer
You assume wrong. Write buffering is only useful when you perform
multiple writes between the WriteBufferOpen() and
WriteBufferClose/Flush() calls, or sending very large data blocks.
But you are not doing that. You are flushing the buffer after each
Write(). Which effectively circumvents what write buffering is all
about.
> I experimented with Disconnect; and Connect; and so on but
> the Open/Close methods of the Socket seemed to be most
> direct to me.
You are not supposed to be using them directly.
> For the binary data connections I created a TReadThread =
> Class(TThread), that is created when connection is opened, and
> terminated/freed when connection is closed.
That is ok to do.
> Sometimes it occurs, that the thread doesn't terminate.
That is because you are not disconnect the socket properly, so the
thread does not detect it so it can abort any operations that are
pending.
> The thread is as easy as this (without exception handling):
Make sure that your exception handler is allowing the thread to
terminate.
> while (Not Terminated) and conn.Socket.Opened do begin
while (not Terminated) and (conn.Connected) do begin
> I := InputBuffer.Size;
> If I>0 then begin
>
> S := ReadString(I);
Use InputBufferAsString() instead:
S := InputBufferAsString;
if S <> '' then
> One time it happened, the tracer showed me the program was in
> CheckForDataSource() method. Is it possible that this methode
> doesn't return?
Indy uses the socket API select() function to implement its timeouts.
In very very rare occasions, select() has been known to freeze up at
the OS level. Byt it is so rare as to not really even be a problem.
> I read about this method is not thread safe.
It is fine, as long as no other thread is trying to read from the same
socket.
Gambit
>
> No. You should not be creating a TIdTCPConnection directly in the
> first place. You should be using a TIdTCPClient instead, ie:
>
Thank you, that is the way I first wanted to do it. Then ran into
problems and tried about every method of connecting and getting data. It
now works better but I got another problem back I hoped to have eliminated:
I now use a TIdTcpClient I create in a self-built TComponent class. This
class has a Connect routine that looks like this:
conn.Host := ...
conn.Port := ...
RIELog('Connect');
conn.Connect;
RIELog is a global Logging routine that adds a line to a Memo box.
In the OnConnected Event Handler there is
RIELog('Connected');
readThread := TReadThread.Create(conn);
readThread.OnRead := Read;
readThread.OnTerminate := ThreadTerminated;
After disconnection I terminate the thread with .Terminate and Free it
in the OnTerminate event handler.
Now my program connects, disconnects, connects, disconnects.. and on the
second or third or even fifth attempt, it deadlocks. The last Log entry
is Connected and debugger is in CPU mode with no call stack or links to
ntdll.RtlTryEnterCriticalSection. I don't understand why, and debug info
makes the .Connect method of the TcpClient suspicious.
The class methods run in main thread context, except the OnRead handler
which is triggered by the read thread. Another thread is holding another
TCP Connection with only Write and ReadLn commands.
Any tips?
Michael Stieler
> conn.Connect;
Again a small addition... if I .Free and .Create the TcpClient every
time before calling .Connect there seems to be no dead-lock. Maybe I do
something wrong on disconnect? The error never occurs on the first connect.
I use conn.Disconnect and in the OnDisconnected event handler:
if Assigned(readThread) then
readThread.Terminate; // FreeOnTerminate := true
If Assigned(conn.IOHandler) then
conn.IOHandler.InputBuffer.Clear;
// Notify application (fire event)
If Assigned(FOnDisconnect) then
FOnDisconnect(self);
I don't use WriteBuffering.
Thanks,
Michael Stieler
Sorry for spamming... but the dead-lock still occurs. Just took longer now.
Regards,
Michael
> It now works better but I got another problem back I hoped to have
eliminated:
Which is what exactly?
> After disconnection I terminate the thread with .Terminate
Are you calling WaitFor(), though? You should be.
> and Free it in the OnTerminate event handler.
Do not do that! That is not a valid place to free the thread. That
is a good way to crash the VCL and deadlock other threads. Just free
the thread after WaitFor() has exited instead, ie:
constructor TMyComp.Create(AOwner: TComponent);
begin
conn := TIdTCPClient.Create(Self);
end;
procedure TMyComp.Connect;
begin
conn.Host := ...
conn.Port := ...
RIELog('Connecting');
conn.Connect;
try
readThread := TReadThread.Create(conn);
readThread.OnRead := Read;
readThread.Resume;
except
conn.Disconnect;
raise;
end;
RIELog('Connected');
end;
procedure TMyComp.Disconnect;
begin
RIELog('Disconnecting');
if readThread <> nil then readThread.Terminate;
try
conn.Disconnect;
finally
if readThread <> nil then
begin
readThread.WaitFor;
FreeAndNil(readThread);
end;
end;
RIELog('Disconnected');
end;
Gambit
> if Assigned(readThread) then
> readThread.Terminate; // FreeOnTerminate := true
Don't use FreeOnTerminate in this situation.
Gambit
>
>> After disconnection I terminate the thread with .Terminate
>
> Are you calling WaitFor(), though? You should be.
>
>> and Free it in the OnTerminate event handler.
>
> Do not do that! That is not a valid place to free the thread. That
> is a good way to crash the VCL and deadlock other threads. Just free
> the thread after WaitFor() has exited instead, ie:
Okay, I described this wrong.. but the point I'll try to build in is the
order of commands.. 1. Terminate 2. Disconnect 3. WaitFor 4. Free
What if the read thread finishes because its own while condition not
met? Does it harm to call .Terminate on it after this when
.FreeOnTerminate = false? I once had the impression that .WaitFor didn't
return if the thread is already finished. But why shouldn't it..
>
> constructor TMyComp.Create(AOwner: TComponent);
> begin
> conn := TIdTCPClient.Create(Self);
> end;
>
> procedure TMyComp.Connect;
> begin
> conn.Host := ...
> conn.Port := ...
> RIELog('Connecting');
> conn.Connect;
> try
> readThread := TReadThread.Create(conn);
> readThread.OnRead := Read;
> readThread.Resume;
> except
> conn.Disconnect;
> raise;
> end;
> RIELog('Connected');
> end;
You wanted to put the conn.Connect inside the try..except block, didn't
you? Then I'd understand it.
>
> Gambit
>
>
Thank you, I'll try to fix it on Monday and report how it worked.
Michael
> What if the read thread finishes because its own while
> condition not met?
It does not matter why the thread exits from its Execute() method.
Once Execute() has exited, the thread is effectly finished and
WaitFor() can then return immediately afterwards.
> Does it harm to call .Terminate on it after this when
.FreeOnTerminate = false?
All the Terminate() method does it sets the Terminated property to
true. It does not do anything else.
> I once had the impression that .WaitFor didn't return if the
> thread is already finished.
Yes, it does.
> You wanted to put the conn.Connect inside the try..except block,
> didn't you?
No, I did not. I intentionally left it out. If Connect() fails, an
exception is raised. There is no point in creating the thread when
that happens, and the code snippet I showed you is not providing any
other way of reporting whether Connect() failed, so I am letting the
exception escape into the calling code for the application to handle
as it sees fit. The only reason to put Connect() inside the
try..except block would be if the Connect() method had a Boolean
return value instead, ie:
function TMyComp.Connect: Boolean;
begin
Result := False;
conn.Host := ...
conn.Port := ...
try
conn.Connect;
try
readThread := TReadThread.Create(conn);
readThread.OnRead := Read;
readThread.Resume;
except
conn.Disconnect;
raise;
end;
Result := True;
except
end;
end;
Gambit
My read thread Execute procedure:
procedure TReadThread.Execute;
var
S : String;
Begin
try
while (Not Terminated) and conn.Connected do begin
try
conn.IOHandler.CheckForDataOnSource(600);
S := conn.IOHandler.InputBufferAsString;
If S <> '' then
If Assigned( FOnRead ) then
FonRead(conn, S);
except
// Something went wrong, exit loop
break;
end;
End;
except
// catch thread exceptions to avoid access violations
end;
end;
And it fails in the Disconnect procedure:
procedure TModuleProxy.Disconnect;
begin
RIELog('(MP) Before Thread Termination');
if readThread<>nil then begin
readThread.Terminate;
readThread.WaitFor;
FreeAndNil(readThread);
end;
RIELog('After Thread Termination');
// this log is never done and readThread is not nil!
try
conn.Disconnect;
except
// ignore
end;
end;
Any ideas why this could still fail? On a new .Connect() readThread is
not nil and sometimes the program freezes.
Michael
> Any ideas why this could still fail? On a new .Connect() readThread is
> not nil and sometimes the program freezes.
>
> Michael
Oops.. FreeOnTerminate was still true, with false this part works as
expected. Thanks!
Michael