Yaron 53 Posted April 23, 2019 (edited) I am trying to create a really simple TCP/IP connection to a server using Indy on Android. All the communication between the client and server is text based. I tried adjusting the code on Remi's answer from this thread to my needs: https://stackoverflow.com/questions/17372366/delphi-indy-tidtcpclient-reading-data However, what happens is that "FConn.IOHandler.ReadLn;" is blocking (at least under Android), so when the App closes and I try to terminate the thread, calling the thread's "WaitFor" method triggers a freeze as the thread never leaves the blocking "ReadLn" and never terminates. Is there a way to instantly stop ReadLn from waiting so the thread could terminate cleanly? Or should I use "FConn.IOHandler.ReadBytes" instead? P.S. I read this thread: https://stackoverflow.com/questions/12507677/terminate-a-thread-and-disconnect-an-indy-client And in it Remy writes that "Disconnecting the client will cause ReadLn() to raise an exception inside the thread.", which is not the case for me under Android (I'm trying to catch the exception and write a log entry when it triggers, but nothing gets written). I also tried wrapping the disconnect method in a "try except" block, but it's not triggering any exception. The code I'm using: procedure TMainForm.CreateTCPIPConnection; var B : Boolean; begin IdTCPClient.Host := AddressEdit.Text; IdTCPClient.Port := StrToIntDef(PortEdit.Text,4769); B := False; Try IdTCPClient.Connect; except on E: Exception do Begin B := True; {$IFDEF TRACEDEBUG}AddDebugEntry('TCP/IP connect exception : '+E.Message);{$ENDIF} End; end; If B = False then Begin If Assigned(ZPReadThread) = False then try ZPReadThread := TReadingThread.Create(IdTCPClient); ZPReadThread.OnData := DataReceived; ZPReadThread.Start; except on E: Exception do Begin IdTCPClient.Disconnect; {$IFDEF TRACEDEBUG}AddDebugEntry('TCP/IP exception creating read thread : '+E.Message);{$ENDIF} End; end; End; end; procedure TReadingThread.Execute; begin {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread created');{$ENDIF} while not Terminated do begin {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread ReadLn (before)');{$ENDIF} Try FData := FClient.IOHandler.ReadLn; Except on E: Exception do Begin {$IFDEF TRACEDEBUG}AddDebugEntry('TCP/IP IOHandler.ReadLn exception : '+E.Message);{$ENDIF} End; End; //FClient.IOHandler.ReadBytes(AData, sizeof(TWaveFormSample), False); {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread ReadLn (after)');{$ENDIF} if (FData <> '') and Assigned(FOnData) then Synchronize(DataReceived); //Sleep(1); end; {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread about to terminate');{$ENDIF} end; procedure TMainForm.FormClose(Sender: TObject; var Action: TCloseAction); begin If IdTCPClient.Connected = True then Begin try IdTCPClient.Disconnect; except on E: Exception do Begin {$IFDEF TRACEDEBUG}AddDebugEntry('TCP/IP Disconnect exception : '+E.Message);{$ENDIF} End; end; End; if Assigned(ZPReadThread) then Begin {$IFDEF TRACEDEBUG}AddDebugEntry('Terminating Read thread');{$ENDIF} ZPReadThread.Terminate; {$IFDEF TRACEDEBUG}AddDebugEntry('Waiting for read thread termination');{$ENDIF} ZPReadThread.WaitFor; {$IFDEF TRACEDEBUG}AddDebugEntry('Finished waiting for read thread termination');{$ENDIF} FreeAndNil(ZPReadThread); End; end; Edited April 23, 2019 by Yaron Share this post Link to post
Remy Lebeau 1396 Posted April 23, 2019 (edited) TIdTCPClient.Connect() is a blocking operation, so you should not be calling it in your UI thread. Call it in a worker thread as well. I would suggest changing your reading thread to call Connect() before entering its reading loop, and then call Disconnect() when the loop is finsihed. Also, do not call TIdTCPClient.Connected() in a different thread than TIdIOHandler.ReadLn(). Connected() performs a read operation, so it may read bytes that ReadLn() is expecting, or worse, having 2 threads reading from the same socket may cause the TIdIOHandler.InputBuffer to store the read bytes in the wrong order. Never perform reading operations across thread boundaries, unless you synchronous the threads. In any case, the demo you based your code on was written for Delphi 2007, which only supports Windows development. On non-Windows platforms (Android is Java running on top of Linux), closing a socket in one thread is not *guaranteed* to abort a blocking socket operation in another thread (though it *should* since Indy shuts down the socket before closing it, and a shut down should abort a socket operation in progress). However, on Nix-based systems at least, where sockets are just file descriptors, it is *possible* that the act of closing a socket descriptor allows that descriptor to be reused right away for another socket, or even a file, thus you *might* be performing data I/O on something that you are not expecting. In your case, a simple fix I would suggest for you is to call ReadLn() with a timeout (either via its own ATimeout input parameter, or via the TIdIOHandler.ReadTimeout property), and then you can check the TIdIOHandler.ReadLnTimedOut property whenever ReadLn() exits, if needed. Then you can simply signal your thread to terminate, and it will do so when the current timeout elapses and control returns to the 'while' loop. Try this: procedure TMainForm.CreateTCPIPConnection; begin if not Assigned(ZPReadThread) then begin IdTCPClient.Host := AddressEdit.Text; IdTCPClient.Port := StrToIntDef(PortEdit.Text, 4769); try ZPReadThread := TReadingThread.Create(IdTCPClient); try ZPReadThread.OnData := DataReceived; ZPReadThread.Start; except FreeAndNil(ZPReadThread); raise; end; except on E: Exception do begin {$IFDEF TRACEDEBUG}AddDebugEntry('TCP/IP exception creating read thread : '+E.Message);{$ENDIF} end; end; end; end; procedure TReadingThread.Execute; begin {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread created');{$ENDIF} try FClient.ConnectTimeout := 10000; // <-- use whatever you want... FClient.Connect; except on E: Exception do begin {$IFDEF TRACEDEBUG}AddDebugEntry('TCP/IP connect exception : '+E.Message);{$ENDIF} raise; end; end; try FClient.IOHandler.ReadTimeout := 5000; // <-- use whatever you want... while not Terminated do begin {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread ReadLn (before)');{$ENDIF} try FData := FClient.IOHandler.ReadLn; except on E: Exception do begin {$IFDEF TRACEDEBUG}AddDebugEntry('TCP/IP IOHandler.ReadLn exception : '+E.Message);{$ENDIF} raise; end; end; //FClient.IOHandler.ReadBytes(AData, sizeof(TWaveFormSample), False); {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread ReadLn (after)');{$ENDIF} if (FData <> '') and Assigned(FOnData) then Synchronize(DataReceived); //Sleep(1); end; finally FClient.Disconnect; end; end; procedure TReadingThread.DoTerminate; begin {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread terminating');{$ENDIF} inherited; end; procedure TMainForm.FormClose(Sender: TObject; var Action: TCloseAction); begin if Assigned(ZPReadThread) then begin {$IFDEF TRACEDEBUG}AddDebugEntry('Terminating Read thread');{$ENDIF} ZPReadThread.Terminate; {$IFDEF TRACEDEBUG}AddDebugEntry('Waiting for read thread termination');{$ENDIF} ZPReadThread.WaitFor; {$IFDEF TRACEDEBUG}AddDebugEntry('Finished waiting for read thread termination');{$ENDIF} FreeAndNil(ZPReadThread); end; end; Another option would be to have your UI thread shut down the socket directly, thus aborting the read. Let the reading thread close the socket when ready. procedure TMainForm.CreateTCPIPConnection; begin if not Assigned(ZPReadThread) then begin IdTCPClient.Host := AddressEdit.Text; IdTCPClient.Port := StrToIntDef(PortEdit.Text, 4769); try ZPReadThread := TReadingThread.Create(IdTCPClient); try ZPReadThread.OnData := DataReceived; ZPReadThread.Start; except FreeAndNil(ZPReadThread); raise; end; except on E: Exception do begin {$IFDEF TRACEDEBUG}AddDebugEntry('TCP/IP exception creating read thread : '+E.Message);{$ENDIF} end; end; end; end; procedure TReadingThread.Execute; begin {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread created');{$ENDIF} try FClient.ConnectTimeout := 10000; // <-- use whatever you want... FClient.Connect; except on E: Exception do begin {$IFDEF TRACEDEBUG}AddDebugEntry('TCP/IP connect exception : '+E.Message);{$ENDIF} raise; end; end; try while not Terminated do begin {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread ReadLn (before)');{$ENDIF} try FData := FClient.IOHandler.ReadLn; except on E: Exception do begin {$IFDEF TRACEDEBUG}AddDebugEntry('TCP/IP IOHandler.ReadLn exception : '+E.Message);{$ENDIF} raise; end; end; //FClient.IOHandler.ReadBytes(AData, sizeof(TWaveFormSample), False); {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread ReadLn (after)');{$ENDIF} if (FData <> '') and Assigned(FOnData) then Synchronize(DataReceived); //Sleep(1); end; finally FClient.Disconnect; end; end; procedure TReadingThread.DoTerminate; begin {$IFDEF TRACEDEBUG}AddDebugEntry('Read thread terminating');{$ENDIF} inherited; end; type TIdStackBSDBaseAccess = class(TIdStackBSDBase) end; procedure TMainForm.FormClose(Sender: TObject; var Action: TCloseAction); begin if Assigned(ZPReadThread) then begin {$IFDEF TRACEDEBUG}AddDebugEntry('Terminating Read thread');{$ENDIF} ZPReadThread.Terminate; try {$IFDEF TRACEDEBUG}AddDebugEntry('Shutting down socket');{$ENDIF} TIdStackBSDBaseAccess(GBSDStack).WSShutdown(IdTCPClient.Socket.Binding.Handle, Id_SD_Both); finally {$IFDEF TRACEDEBUG}AddDebugEntry('Waiting for read thread termination');{$ENDIF} ZPReadThread.WaitFor; {$IFDEF TRACEDEBUG}AddDebugEntry('Finished waiting for read thread termination');{$ENDIF} FreeAndNil(ZPReadThread); end; end; end; Edited April 23, 2019 by Remy Lebeau 1 Share this post Link to post
Yaron 53 Posted April 24, 2019 I am writing a remote control App that controls a PC. I am sending commands over TCP/IP each time a user clicks a UI button and at the same time, listening to responses to my commands. Presently, I am getting responses in the background "reading" thread and sending the command in the main thread. After reading your comments I realize my issue is further complicated, as your recommend not calling any TCP/IP function at all in my main thread, which means my Read function can't be blocking at all, otherwise the user will experience input-lag. Is it safe to call "IdTCPClient.IOHandler.WriteLn" in the main thread? If not, what is the best approach to check if there is data waiting without blocking? Share this post Link to post
Lars Fosdal 1792 Posted April 24, 2019 My favorite approach is to have a connection thread handler that implements the "protocol" for the connection - using a input queue and an output queue to communicate with the owning thread, which in this case could be the main thread. This pattern is often called using mailboxes. I.e. the main thread posts a "task" to the thread input queue aka inbox, the thread loops with a small sleep to poll the inbox and deals with connects/disconnects and sends whatever needs to be sent over Indy to the remote host, collects the answer, and places it in the output queue aka outbox. The outbox can be polled in the main thread, or you can design the queue to post a message to the main thread, which then polls the outbox on receving the message. Protocol errors can also be posted as outbox events. 4 Share this post Link to post
Remy Lebeau 1396 Posted April 24, 2019 (edited) 6 hours ago, Yaron said: Presently, I am ... sending the command in the main thread. I don't recommend that model, ESPECIALLY on Android, which DOES NOT ALLOW you to perform socket operations on the main UI thread if you are targeting API level 11 (Android 3.0 Honeycomb) or later. It will throw a NetworkOnMainThreadException exception if you try. Quote After reading your comments I realize my issue is further complicated, as your recommend not calling any TCP/IP function at all in my main thread, which means my Read function can't be blocking at all, otherwise the user will experience input-lag. Not only that, but Android will likely just kill your app process outright if the main thread becomes blocked for too long. Or, at the very least, it will prompt the user to kill the process. Quote Is it safe to call "IdTCPClient.IOHandler.WriteLn" in the main thread? "safe" is relative to the platform you are running on. But I would not recommend it, since it has the POTENTIAL to block the main thread, which may be acceptable for desktop platforms like Windows, but is certainly not acceptable for mobile platforms like iOS and Android. Quote If not, what is the best approach to check if there is data waiting without blocking? You can call it in a worker thread and let it block normally. But, if you really want to check manually, the IOHandler does have InputBufferIsEmpty() and CheckForDataOnSource() methods, where the latter has a timeout parameter available that you can set as low as 0ms. Edited April 24, 2019 by Remy Lebeau 1 Share this post Link to post