Hi,

I'm trying to close a socket (full code here:
https://bitbucket.org/silvioprog/tcpipcomp/src/6b43739d5416424e82112b858a9b71d26c8c6165/src/tcpipserver.pas?at=master ):

destructor TTcpIpServerSocketThread.Destroy;
begin
  FSocket.StopAccepting;
  FSocket.Free;
  inherited Destroy;
end;

But, it returns:

"Could not accept a client connection on socket: 288, error 10004"

The erros occurs just when I run my project on IDE. I noticed that error is in this code:

Function TInetServer.Accept : Longint;
Var l : longint;
begin
  L:=SizeOf(FAddr);
  Result:=Sockets.fpAccept(Socket,@Faddr,@L);
  If Result<0 then
{$ifdef Unix}
    If SocketError=ESysEWOULDBLOCK then
      Raise ESocketError.Create(seAcceptWouldBlock,[socket])
    else
{$endif}
      Raise ESocketError.Create(seAcceptFailed,[Socket,SocketError]);
end;

The "Result" is returning -1, so, what I must do to it return 0? Overriding in a descendant class of TInetServer?

​After termintate you need to wait the thread to stop, WaitFor here the function.
Search for WaitFor​ in
https://sourceforge.net/p/minilib/source/ci/master/tree/socket/source/mnServers.pas

2013/5/2 Zaher Dirkey

Yes, but in this case, WaitFor blocks my app.

silvioprog wrote

Yes, but in this case, WaitFor blocks my app.

​Yes, but before terminate your socket/client connections, you need also close the handle of it, or stop receive, it will return immateriality without receive (with error), so all connections will drop, and you wait to finish that.​

2013/5/2 Zaher Dirkey

Yes. Please see full code here:

https://bitbucket.org/silvioprog/tcpipcomp/src/46420ac50800b015e7f17b91f019d4a3230d04c3/src/tcpipserver.pas?at=master

Lines 277 and 278. That is, I already do that. The problem now is how to stop the "Accept" but without errors.

I implemented the "Stop" method in socket, but I did not want to do it this ugly way.

2013/5/2 silvioprog

I tested current code on Linux, but unfortunately when I try to close, the app stays locked. :|

silvioprog wrote

I tested current code on Linux, but unfortunately when I try to close, the app stays locked. :|

​Yes headache :P​

​Do not close, i use Shutdown

function TmnSocket.DoShutdown(How: TmnShutdown): TmnError;
const
  cHow: array[TmnShutdown] of Integer = (0, SHUT_RD, SHUT_WR, SHUT_RDWR);
var
  c: Integer;
begin
  CheckActive;
  c := fpshutdown(FHandle, cHow[How]);
  if c = SOCKET_ERROR then
  begin
    Result := erFail;
//    RaiseLastOSError; do not raise an error, maybe it is disconnected by
the other side
  end
  else
    Result := erNone;
end;
​

silvioprog wrote

Lines 277 and 278. That is, I already do that. The problem now is how to stop the "Accept" but without errors.

Using linux (or some other unix like thingie), you could cancel the thread using pthread_cancel(), the call will then return immediately with a specific (can't remember which) error code in errno (socketerror in this case I believe).

Problem with pthread_cancel is that there are quite a lot of cancelation points, see http://stackoverflow.com/questions/433989/posix-cancellation-points , so be careful with this approach.

Another way would be to use a non-blocking socket to accept connections...

Ewald wrote

pthread_cancel()

pthread_cancel()
​ ​
​ not clos
​e​
the handles i though​.​

2013/5/2 Zaher Dirkey

I tried with fpshotdown and closesocket, like this:

https://github.com/graemeg/freepascal/blob/master/packages/fcl-web/src/base/custfcgi.pp#L614

But, same error.

This is not the most serious problem. My dilemma now is to close the app. I hate working with threads. -.-'

2013/5/2 silvioprog

I tried with fpshotdown ...

fpshutdown...

Same solution as for recv and send. Or use blocking mode with a select before accept, or use non blocking mode and deal with EAGAIN or EWOULDBLOCK. Here you seem to use blocking mode and accept will only return with a new connection or an error.

Ludo

That's true, but at least it returns control to you (= the programmer), so you can close the handles manually.

Ewald wrote

That's true, but at least it returns control to you (= the programmer), so you can close the handles manually.

​I am notprefer force to close any thing, that will make more bugs in your application​, while it is work fine with other programmesr without using cancel it.

Best Regards
Zaher Dirkey

2013/5/3 Ludo Brands

Could you give me an example in practice? Theoretically I understand, but I don't know do in practice. :/

Now, TTcpIpServer is working well. The new problem is: when I close the server with one or more clients connected, I got a memory leak.

Neither do I, but wasn't that what Silvio asked? [by saying: `The problem now is how to stop the "Accept" but without errors`]

I admit, killing an entire thread is a bit overkill for just getting back control from accept, but I also recall that he wanted to close the application [or am I seeing ghosts?]. But if you don't want to use pthread_cancel(), use non-blocking sockets -- In my experience though, it isn't worth the code if you just want to kill the application gracefully.

Anyway, I'm sure there are other ways to get this done (signals?), but these are two methods I know about. Each has it's bonuses and pitfalls.

silvioprog wrote

when I close the server with one or more clients connected, I got a memory leak.

​Check if the thread freed​?
​Try to stop server manually (by button or in Close event in the form)​

Best Regards
Zaher Dirkey

Ewald wrote

"Accept" but without errors

​You cant, Accept give you a handle of new socket or give you an error, not all is fatal error, just check what is it, if it a closed handle or shutdown.​

Best Regards
Zaher Dirkey

2013/5/3 Zaher Dirkey

Yes, I already do it.

Now I'm trying to terminate all unterminated clients:

destructor TTcpIpServerSocketThread.Destroy;
begin
  DoTerminateClients;
  FClients.Free;
  FSocket.Free;
  inherited Destroy;
end;

....

procedure TTcpIpServerSocketThread.DoTerminateClients;
var
  VClients: TList;
  VClient: Pointer;
  VThread: TTcpIpServerClientThread;
begin
  VClients := FClients.LockList;
  try
    for VClient in VClients do
    begin
      VThread := TTcpIpServerClientThread(VClient);
      if Assigned(VThread) and not VThread.Finished then
      begin
        VThread.FreeOnTerminate := False;
        VThread.Terminate;
        FreeAndNil(VThread.FClientSocket);
        VThread.WaitFor;
        FreeAndNil(VThread);
      end;
    end;
  finally
    FClients.UnlockList;
  end;
end;

I'm testing it, on Windows it worked fine, I'll test on Linux now...

Once upon a time, Zaher Dirkey said:

Yes, I know that.

The point being that the answer to one of the OP sub-problems [the one quoted _partially_ above], can be to either use:
- pthread_cancel() [overkill, I know, but nonetheless a solution]
- non-blocking sockets [not very code-efficient IMHO if you only want to exit an application gracefully (high code versus ouput ratio)]
- something different (one can try experimenting with shutting down the listening socket, using signals, ... but I haven't done so, so I can't give any feedback on this)

In our previous episode, Ewald said:

Select on the main socket before doing the accept?

Fixed all errors:

https://bitbucket.org/silvioprog/tcpipcomp/src/273b28b52b30621ffb4955e1136f8a06c160d20a/src/tcpipserver.pas?at=master

I'll test this class severely.

Marco van de Voort wrote

Select on the main socket before doing the accept?

​No i am not use Select before Accept, accept is blocking the thread.

Best Regards
Zaher Dirkey

Yep, possibly the most simple and effective recipe (not much code to add, no radical changes, no overkill). Shame on me for not thinking of that ;-)

In our previous episode, Zaher Dirkey said:

That's the point. Don't. Use a select, it has a timeout, and accept only if select shows activity on the listened to socket.

Marco van de Voort wrote

That's the point. Don't. Use a select, it has a timeout, and accept only if select shows activity on the listened to socket.

​Not sure, but i believe i used it in the past and have problems with it, We can leave it to silvio to test it :P​

​I use "Select"​ for incoming Data not for incoming client connections!

Best Regards
Zaher Dirkey

It doesn't matter. A few facts:
- You create a listening socket which you pass as an argument to accept().
- A socket is a file descriptor.
- Select operates on file descriptors. Read the man page of select [http://linux.die.net/man/2/select] and you'll see that the exact purpose of select is to query whether or not the next IO operation is going to block.

Now that we have established these two facts, I believe that using select before accept to check whether the latter will block is valid logic, wouldn't you say?

PS: The exact details of this implementation I do not know, I haven't tested.

Take a and TBlockSocket.CanRead in
synapse on how to set up select correctly. Then do something like

repeat
  if Socket.CanRead(timeout) then
    begin
    insock:=Socket.Accept;
    if insock-1 then
      //start working with insock
    else
      //deal with error
    end;
until ThreadTerminated;

Ludo

2013/5/4 Ludo Brands

Thank you!

​If you success, tell us, i must fix my sockets too :)​

​I have put Select before Accept and it doesn't work, it blocked forever​ without accepting any connection.
Accept function who block the thread and get new connection.

Best Regards
Zaher Dirkey

Can you show the code you use? You tested on Windows, linux, Mac, ...?

Ludo

Ludo Brands wrote

Can you show the code you use? You tested on Windows, linux, Mac, ...?

​Only on Windows XP

source here
https://sourceforge.net/p/minilib/source/ci/master/tree/socket/source/mnServers.pas about line 323
i changed it to similar to this, but not committed it

      begin
        if Socket.Select(10000, slRead) = erNone then
​ <--- Blocked here forever​
          aSocket := Socket.Accept
        else
          aSocket := nil;
        Enter;

​select function here DoSelect line 132
https://sourceforge.net/p/minilib/source/ci/master/tree/socket/source/mnWinSockets.pas ​
I am using last revision of Lazarus, FPC 2.6 on Windows XP SP3

Best Regards
Zaher Dirkey

But mnServer is works with me for a years, and there is mnHttpServer demo and it is work fine too without Select before Accept https://sourceforge.net/p/minilib/source/ci/master/tree/socket/demo/lazarus/

This is a small test program that works for me on XP and linux:

program testselect;
{$mode objfpc}{$H+}
uses
  Classes,sockets,
  {$ifdef unix}
  unix,baseunix,unixtype
  {$else}
  ctypes,WinSock2
  {$endif}
  ;
CONST
  LISTENPORT=1234;
var
  sock: cint;
  sa,clsa:sockaddr;
  tv:TTimeVal;
  readfds,writefds,exceptfds:tfdset;
  c: Integer;
  res: cint;
  len: tsocklen;
  one: Integer;
begin
  sock:=fpsocket(AF_INET, SOCK_STREAM, 0);
  if sock=-1 then
    begin
    writeln('socket failed with ',socketerror);
    exit;
    end;
  one:=1;
  if fpsetsockopt(sock, SOL_SOCKET, SO_REUSEADDR, @one, sizeof(one))=-1 then
    begin
    writeln('setsockopt failed with ',socketerror);
    exit;
    end;
  fillchar(sa,sizeof(sa),0);
  sa.sin_port:=htons(LISTENPORT);
  sa.sin_family:=AF_INET;
  sa.sin_addr.s_addr:= INADDR_ANY;
  if fpbind(sock,@sa,sizeof(sa))=-1 then
    begin
    writeln('bind failed with ',socketerror);
    exit;
    end;
  if fplisten(sock,10)=-1 then
    begin
    writeln('listen failed with ',socketerror);
    exit;
    end;
  c:=1;
  repeat
    tv.tv_sec:=1;
    tv.tv_usec:=0;
    {$ifdef unix}
    fpFD_ZERO(readfds);
    fpFD_ZERO(writefds);
    fpFD_ZERO(exceptfds);
    fpFD_SET(sock,readfds);
    res:=fpselect(sock+1,@readfds,@writefds,@exceptfds,@tv);
    if (res=-1) and (socketerrorESYSEINTR) then
        begin
        writeln('select failed with ',socketerror);
        exit;
        end;
    if (res>0) and (fpFD_ISSET(sock,readfds)0) then
      begin
    {$else}
    FD_ZERO(readfds);
    FD_ZERO(writefds);
    FD_ZERO(exceptfds);
    FD_SET(sock,readfds);
    res:=select(sock+1,@readfds,@writefds,@exceptfds,@tv);
    if (res=-1) and (socketerrorWSAEINTR) then
       begin
       writeln('select failed with ',socketerror);
       exit;
       end;
    if (res>0) and (FD_ISSET(sock,readfds)) then
      begin
    {$endif}
      fillchar(clsa,sizeof(clsa),0);
      len:=sizeof(clsa);
      if fpaccept(sock,@clsa,@len)=-1 then
        begin
        writeln('accept failed with ',socketerror);
        exit;
        end;
      writeln('connection accepted');
      c:=c+1;
      end;
  until c=2;
  closesocket(sock);
end.

If you run the program and do from a console a
telnet localhost 1234
to connect to the program, it will print 'connection accepted' and close immediately.

Ludo

2013/5/4 Zaher Dirkey

Yes:

https://bitbucket.org/silvioprog/tcpipcomp/src/945866300819bfe0d9d957c64f301e2ff92682e5/src/tcpipserver.pas?at=master#cl-161

2013/5/4 Ludo Brands

I think you should make a server and client component and send it to the Free Pascal team. :)

Hmm there is diffrents
Yours
res:=select(sock+1,@readfds,@writefds,@exceptfds,@tv); Mine
c := WinSock2.select(0, PSetRead, PSetWrite, nil, @TimeVal);

0 here in mine, in yours sock+1 (FHandle+1 in mine). I will take more testing.

According to
http://msdn.microsoft.com/en-us/library/windows/desktop/ms740141(v=vs.85).aspx :
"nfds [in] Ignored. The nfds parameter is included only for compatibility with Berkeley sockets."

I wrote the code for linux and then "ported" it to windows.

However, in your code you do
if (c = 0) or (c = SOCKET_ERROR) then
  begin
    Error;

which is not correct. c=0 is the result of select returning after a timeout. It is not an error.
The fact that

   if Socket.Select(10000, slRead) = erNone then

is blocked forever​ is not normal. It should at least return after 10 seconds.

Ludo

​I am not testing in linux it is broked with me and i have not good internet to fix it.

now in this example
http://msdn.microsoft.com/en-us/library/windows/desktop/ms737526%28v=vs.85%29.aspx They not use Select before acce​pt

and in
http://msdn.microsoft.com/en-us/library/windows/desktop/ms740141%28v=vs.85%29.aspx

The parameter *readfds* identifies the sockets that are to be checked for

readability. If the socket is currently in the
*listen*<http://msdn.microsoft.com/en-us/library/windows/desktop/ms739168%28v=vs.85%29.aspx>state, it will be marked as readable if >an incoming connection request has been received such that an
*accept*<http://msdn.microsoft.com/en-us/library/windows/desktop/ms737526%28v=vs.85%29.aspx>is guaranteed to complete
*without blocking*.

I still not sure in windows need Select, but maybe in Linux only, but i can't test it there.

Zaher Dirkey wrote

now in this example http://msdn.microsoft.com/en-us/library/windows/desktop/ms737526%28v=vs.85%29.aspx They not use Select before acce​pt

If you are happy with accept blocking or if you use non-blocking sockets you don't need select before accept.

That is exactly the purpose of select. The timeout guarantees that your program stays alive and can do something else.

I still not sure in windows need Select, but maybe in Linux only, but i can't test it there.

I repeat, you don't have to use select in Windows or Linux. If you prefer blocking sockets you don't need select but accept/recv/send will block. If you use non-blocking sockets you need to deal with EAGAIN or EWOULDBLOCK (WSAEWOULDBLOCK on windows). There are also alternatives to select: poll, epoll, libevent, etc., etc. on linux, WSAPoll on windows Vista and later. For completeness, on windows you have also the asynchronous overlapped IO mode which is used a lot in high performance servers but rather complex to program.

Ludo

2013/5/5 Ludo Brands

Ludo, I'm almost by completing the units in this link:

https://bitbucket.org/silvioprog/tcpipcomp (this repository is temporary)

The current code (I'm updating it daily) in these units is good? I'm open to suggestions, and we can change whatever it takes.

I want to send these units to the Free Pascal team and a package to the Lazarus team.

Sorry for my English please.

In TTcpIpClientSocketThread.Execute you do a read followed by a select. The logic is: first select then read when something is available.

The sleep(0) is not needed. Select suspends already the thread when waiting.

          VTimeVal.tv_sec := 0;
          VTimeVal.tv_usec := 100;

That is 100 microseconds which is very short. On top of that, when the timeout expires, you test if FDisconnectClient is set and, if not set, you do the read immediately again. The read is therefor again hanging until something comes in.

Watch out for Synchronize(@DoReceive) in a recv loop. That is going to drag down your performance considerably.

Ludo

Ludo Brands wrote

Watch out for Synchronize(@DoReceive) in a recv loop. That is going to drag down your performance considerably.

​Maybe you do need to call synh, leave it to the user to add it in his event, or make it as an option.

For that i hate using components as GUI components, no events in my classes.

2013/5/6 Ludo Brands

In TTcpIpClientSocketThread.Execute you do a read followed by a select. The logic is: first select then read when something is available.

Sorry, but I don't understand well this part. :/ In the old code I not used select, and now I'm using it just to know that recv received data, but without lock the application.
But if there is how to improve, feel free buddy, I noticed that you know well about sockets. :)

I can add your SSH key on my Bitbucket account and you can commit directly in this code. Or, if your prefer, you can send me GIT patches. Feel free again.

The sleep(0) is not needed. Select suspends already the thread when waiting.

Done. I removed that.

VTimeVal.tv_sec := 0; VTimeVal.tv_usec := 100; That is 100 microseconds which is very short.

Is 500us good value for delay?

Yes. I needed it when I made a small HTTP server and I needed to disconnect the client in each request. There are a best way to do it? :/

Watch out for Synchronize(@DoReceive) in a recv loop. That is going to drag down your performance considerably.

I don't understand well this part hehe... Hm... In some tests I did not see decreased performance, please could you show me an example decreasing the performance? I need to reproduce the problem, and debug the code to improve it.

Thank you very much for the explanations again. On the end of it maybe we have a nice multi-threaded component for work with TCP/IP. :)

2013/5/6 Zaher Dirkey

But it already implemented friend, eg:

  if VSynchronized then
    Synchronize(@DoClientReceive)
  else
    DoClientReceive;

Please see all code here: https://bitbucket.org/silvioprog/tcpipcomp/src

For that i hate using components as GUI components, no events in my classes.

There you can see two examples: console (client/server); gui (client/server).