rlebeau on master
Updated Computil.exe compiled b… (compare)
try
with AContext.Connection.IOHandler do
begin
CheckForDataOnSource(10);
if not InputBufferIsEmpty then
begin
RxBufStr := InputBuffer.ExtractToString(-1, IndyTextEncoding_UTF8);
Log(RxBufStr);
AContext.Binding.SendTo(AContext.Binding.PeerIP, 7, RxBufStr, Id_IPv4);
IdBuffer.Extract()
end;
end;
finally
end;
TIdBuffer.ExtractString()
, especially with a text encoding involved. You are reading arbitrary bytes and assuming they constitute a complete sequence of characters, which is not guaranteed, so the bytes may not decode to a string properly. And, you shouldn't be calling Binding.SendTo()
directly at all (and besides, using sendto()
on a TCP/IP socket is meaningless anyway, the destination parameters are ignored so it acts the same as send()
). This kind of code is a good way to bypass TIdTCPServer's ability to auto-stop its client threads. It expects an exception to be raised when the socket is closed, but you are not allowing it to raise anything. The correct thing to do would be to use TIdIOHandler.ReadBytes()
instead, letting it block until new bytes arrive and raise an exception if the client disconnects or the server is being shutting down. And use TIdIOHandler.Write()
instead of Binding.Send/To()
directly.
var
RxBufStr: string;
begin
with AContext.Connection.IOHandler do
begin
CheckForDataOnSource(10);
CheckForDisconnect; // <-- add this
if not InputBufferIsEmpty then
begin
RxBufStr := InputBufferAsString(IndyTextEncoding_8Bit); // <-- don't assume any encoding
Log(RxBufStr);
Write(RxBufStr, IndyTextEncoding_8Bit); // <-- use TIdIOHandler.Write() instead
end;
end;
end;
var
RxBuf: TIdBytes;
begin
with AContext.Connection.IOHandler do
begin
ReadBytes(RxBuf, -1);
Log(RxBuf);
Write(RxBuf);
end;
end;
TIdECHOServer
instead of TIdTCPServer
. For logging, you can assign a TIdLog...
component (TIdLogFile
, TIdLogEvent
, etc) to the AContext.Connection.IOHandler.Intercept
property, such as in the OnConnect
event.
AuthRetries
and ProxyAuthRetries
counters, giving TIdHTTP
more chances to continue authentication attempts. Please don't assume what you think is happening, debug and find out exactly what is really happening.
TIdTCPServer
is to use blocking I/O that follows a defined protocol, raising exceptions on errors/disconnects/shutdowns, and let the server handle the exceptions. If you skip that logic, you become responsible for handling certain things manually, like shutdown. For instance, you could set a variable before setting Active=False
(or, just look at Active
itself, since it is toggled to false before thread shutdowns occur), and then have OnExecute
look at that variable, and if set then call AContext.Connection.Disconnect
, or raise your own exception, before exiting the event.
Log()
itself is deadlocking, such as if it tries to synchronize with the main UI thread while the main UI thread is blocked waiting for the server to finish shutdown. Do not synchronize with the thread that is shutting down the server, that is a guaranteed deadlock. Either skip the synched operation during shutdown, or use a separate worker thread to shutdown the server so syncs can still be processed.
char
and your code is not accounting for that. You can't use UTF-8 unless you ensure complete byte sequences for multi-byte characters, but your original code was not doing that. What kind of protocol is your server implementing? If the text is line-based, for instance, then you could just use TIdIOHandler.ReadLn()
, in which case using UTF-8 would be OK. But if you use ReadBytes(-1)
or ExtractString(-1)
or InputBufferAsString
then all guarantees about the completeness of multi-byte sequences go out the window
@rlebeau this has worked for me and solved my problem:
with IdTCPServer1.Contexts.LockList do
try
for iA := Count - 1 downto 0 do
begin
Context := Items[iA];
if Context = nil then
Continue;
Context.Connection.IOHandler.WriteBufferClear;
Context.Connection.IOHandler.InputBuffer.Clear;
Context.Connection.IOHandler.Close;
if Context.Connection.Connected then
Context.Connection.Disconnect;
end;
finally
IdTCPServer1.Contexts.UnlockList;
IdTCPServer1.Active := False;
end;
What do you think?
This code also suggested by you works very well:
with AContext.Connection.IOHandler do
begin
CheckForDataOnSource(10);
CheckForDisconnect; // <-- add this
if not InputBufferIsEmpty then
begin
RxBufStr := InputBufferAsString(IndyTextEncoding_8Bit); // <-- don't assume any encoding
Log(RxBufStr);
Write(RxBufStr, IndyTextEncoding_8Bit); // <-- use TIdIOHandler.Write() instead
end;
end;
Now my question:
Active
to false, it already disconnects the clients for you, and then waits for their threads to terminate. Your issue is that your code is blocking the threads from terminating correctly. That is what you need to fix properly, not hack around it.
procedure TForm2.Log(const s: string);
begin
mLog.Lines.Add(s);
end;
@elekgeek your Log()
function is directly accessing a UI component (a TMemo
?). THAT IS NOT THREAD-SAFE! That could easily cause deadlocks (amongst many other problems). OnExecute
is fired in a worker thread, so Log()
MUST synchronize with the main UI thread. I suggest it use TThread.Queue()
or TIdNotify
for that purpose (depending on your version of Delphi). That would avoid any cross-thread access issues, avoid any deadlock scenarios, and avoid blocking the OnExecute
code (which should not need to wait on the UI to display log messages). For example:
procedure TForm2.Log(const s: string);
begin
TThread.Queue(nil,
procedure
begin
mLog.Lines.Add(s);
end
);
end;
or:
uses
..., IdSync;
type
TLog = class(TIdNotify)
protected
FMsg: string;
procedure DoNotify; override;
public
constructor Create(const s: string); reintroduce;
end;
constructor TLog.Create(const s: string);
begin
inherited Create;
FMsg := s;
end;
procedure TLog.DoNotify;
begin
Form2.mLog.Lines.Add(FMsg);
end;
procedure TForm2.Log(const s: string);
begin
TLog.Create(s).Notify;
end;
procedure TForm1.FormCreate(Sender: TObject);
var
I: Integer;
begin
BroadcastListenerIPv6.DefaultPort := 6000;
for I := 0 to BroadcastListenerIPv6.Bindings.Count - 1 do
BroadcastListenerIPv6.Bindings.Items[I].Port := BroadcastListenerIPv6.DefaultPort;
BroadcastListenerIPv6.MulticastGroup := 'FF02:0:0:0:0:0:0:1';
BroadcastListenerIPv6.Active := True;
end;
procedure TForm1.FormCreate(Sender: TObject);
var
I: Integer;
begin
BroadcastServerIPv6.Port := 6000;
BroadcastServerIPv6.MulticastGroup := 'FF02:0:0:0:0:0:0:1';
BroadcastServerIPv6.Active := True;
BroadcastListenerIPv6.DefaultPort := 6000;
for I := 0 to BroadcastListenerIPv6.Bindings.Count - 1 do
BroadcastListenerIPv6.Bindings.Items[I].Port := BroadcastListenerIPv6.DefaultPort;
BroadcastListenerIPv6.MulticastGroup := 'FF02:0:0:0:0:0:0:1';
BroadcastListenerIPv6.Active := True;
end;