Indy10 multithread issue

Hello, this is my convesation with Indy10 developer:

@icegood:

We have in TIdIOHandlerStack.Connected
ReadFromSource(False, 0, False);
What indy is supposed to read here? Sometimes (vary rarely) my application based on RemObjects SuperTCPServer hangs there as there is nothing to read from server.

@rlebeau:
Indy uses synchronous socket I/O, so the only way to test if a socket is still “connected” is to perform I/O on it. That is what Connected() is doing when it calls ReadFromSource(). It is checking if there is any inbound data on the socket (and if so, the data is read and put in the InputBuffer for later use), using a 0ms timeout and ignoring any disconnect/timeout errors. It should not be hanging if there is no data, unless the OS itself hangs when Indy calls the socket API select() function (which has been known to happen, despite the timeout - that is an OS issue, not an Indy issue).

@icegood:
your comment makes sense in case if it hangs while calling Readable (ATimeout) in TIdIOHandler.ReadFromSource. But in reality it hangs inside call
LByteCount := ReadDataFromSource(LBuffer); a few lines below. I can imagine next multithread scenario:

  1. first thread actively does IO over socket
  2. second thread checks for connection and for this moment due activity of first thread Readable returns true.
  3. Second thread calls ReadDataFromSource, but for this moment first thread fully has processed data => second thread hangs there.
    Remobject’s SuperTCPServer depends on Indy library and assumes that Indy10 supports multithread processing.
    As for me fix should be in calling of just Readable inside TIdIOHandlerStack.Connected instead of ReadFromSource

@rlebeau:
It is not safe for multiple threads to read from a socket at the same time. That includes calling Connected(). For exactly the reason you mentioned - it gets the socket state out of sync. I/O must be synchronized across threads. It is safe for 1 thread to write while another thread reads, that does not need to be synchronized. But having 2+ threads reading, or 2+ threads writing, must be synchronized. On the other hand, 99% of the time there is no need to call Connected() directly, just let Indy raise an exception during normal I/O operations. That being said, having Connected() call Readable() instead of ReadFromSource() is not good enough, since a socket is marked as readable during a graceful disconnect, an actual read is needed to discover the disconnect.

======================
So i wonder whether newer versions of Remobjects took into account that ‘Connected’ method should be called from worker thread (not from main one as in older versions!). Indy9 simply didn’t have such a verification.

Can you create a simple testcase that reproduces this behavior?
as a temporary workaround, you can use other implementations of SuperTCP: Synapse- or ICS-based components.

My sample would be straightforward from described above scenario:

  1. Create background thread which actively reads from/writes to socket
  2. Create from main thread one more request => this request might hang in ‘connected’ method

Anyway i will attach test case later

Update: and even simpler use case: it’s enough to create many sequential async requests from main thread only.

Thanks, logged as bugs://77709

I can’t reproduce any hangs with above scenario.
I’ve tried with this code:

procedure TClientForm.Button1Click(Sender: TObject);
var
  i: integer;
begin
  if fAsync = nil then fAsync := CoNewService_AsyncEx.Create('supertcp://localhost:8095/bin');
  for i := 0 to 1000 do
    fAsync.BeginGetServerTime(Callback, pointer(i));
end;

testcase.zip (109.4 KB)