You are not logged in.
- Topics: Active | Unanswered
#1 2016-07-13 16:52:07
[Discussion] Close the TCrtSocket in case recv() return 0
While implementing a firefox remote debug protocol we discover an issue in TCrtSocket.TrySockRecv.
If connection is closed by peer recv(..) return a 0 as described here
This is normal situation, and my opinion - we should close a socket.
So, in TCrtSocket.TrySockRecv:
//instead of
if Size<=0 then
exit
//should be
if Size<=0 then begin
if Size=0 then
Close;
exit;
end; In current realization I can't determinate - is socket closed or some errors exists.
AB's answer:
I'm not sure it is correct patch.
If there is a timeout (no data received in the expected period), then Size=0 also, and we definitively should not close the socket.
Please create a forum thread for further discussion.
It is better not to use the tickets for discussion!
Does anyone have any ideas about the right way?
Offline
#2 2016-07-13 18:12:07
- danielkuettner
- Member
- From: Germany
- Registered: 2014-08-06
- Posts: 357
Re: [Discussion] Close the TCrtSocket in case recv() return 0
After reading this:
https://msdn.microsoft.com/de-de/librar … s.85).aspx
I guess, you could/should close the socket.
Offline
#3 2016-07-14 08:21:08
Re: [Discussion] Close the TCrtSocket in case recv() return 0
Seems fine also under Linux:
http://man7.org/linux/man-pages/man2/recv.2.html
So I've committed the patch.
http://synopse.info/fossil/info/655321c360
Offline
#5 2016-07-19 11:40:35
Re: [Discussion] Close the TCrtSocket in case recv() return 0
Last 3 days I fight with a socket timeout problem described [ff91d71434].
But I discover root of my problem not a timeout. On the some Windows network configurations (looks like when VPN adapters is added) calls to HttpGet randomly (1 time work, 3 times not) fail with a connect timeout
initResult := HttpGet(CONTROLLER.serverConfig.HTTPServer.getFullURL + 'init', nil);After rewriting to TWinHTTP
initResult := TWinHTTP.Get(CONTROLLER.serverConfig.HTTPServer.getFullURL + 'init', '', true, nil);everything is OK.
@AB - My suggestion is to modify SynCrtSock.HttpGet to always use a TWinHTTP on Windows (just move a one ifdef up a little). It can save a lot of debugging time for others 
Offline
#7 2016-07-19 12:26:16
Re: [Discussion] Close the TCrtSocket in case recv() return 0
Yes, but I can't provide a detail description of a TCrtSocket problem, so no idea how to fix it. On the servers where problem exists I can't debug, on my developer environments I can't reproduce a problem - I try on 3 different computers w/o results
Last edited by mpv (2016-07-19 12:28:36)
Offline
#8 2016-07-19 12:45:04
Re: [Discussion] Close the TCrtSocket in case recv() return 0
Perhaps some issue at IP/socket level, depending on the TCP/IP stack parameters (and firewall)?
IP exhausting ports?
IP not reusing existing socket?
Did you try keeping the socket alive between requests?
Offline
#10 2020-08-20 13:44:49
Re: [Discussion] Close the TCrtSocket in case recv() return 0
@ab - the issue with socket close detection is again in trunk. I think that for a long time - after AsynchRecv is added , but noticed just now.
In current implementation of TCrtSocket.TrySockRecv - see this link
read := AsynchRecv(fSock,Buffer,read);
if read<=0 then begin // no more to read, or socket issue?
{$ifdef SYNCRTDEBUGLOW}
TSynLog.Add.Log(sllCustom2, 'TrySockRecv: sock=% AsynchRecv=% %',
[Sock,read,SocketErrorMessage],self);
{$endif}
if WSAIsFatalError then begin
Close; // connection broken or socket closed gracefully
exit;
end;
if StopBeforeLength then
break;
end in case connection is closed by counterpart for blocked sockets read result is 0 and WSAIsFatalError is false (WSAGetLastError returns 0), so socket not closing and server continue to wait for data.
The correct fix here is to close a socket in case we receive 0 bytes - as a side effect this remove unnecessary call to WSAGetLastError
if (socketIsBlocking and (read=0) {connection closed}) or WSAIsFatalError then begin
Close; // connection broken or socket closed gracefully
exit;
end; But we need to know socket Is Blocking or not. The question - how can we ensure socket is blocking (without syscall)? May be add a property to TCrtSocket? Or we can expect socket in TCrtSocket.TrySockRecv is always blocking?
Last edited by mpv (2020-08-20 13:47:31)
Offline
#11 2020-08-20 14:15:56
Re: [Discussion] Close the TCrtSocket in case recv() return 0
I guess it is safe to write
if (read=0) or WSAIsFatalError then beginEven of Windows, IIRC, it means end of file, i.e. "if the connection has been gracefully closed, the return value is zero"
- see https://docs.microsoft.com/en-us/window … nsock-recv and https://man7.org/linux/man-pages/man2/read.2.html
On non-blocking sockets, it returns -1 and an error code as EAGAIN or EWOULDBLOCK.
Offline
#12 2020-08-20 14:26:47
Re: [Discussion] Close the TCrtSocket in case recv() return 0
Please check https://synopse.info/fossil/info/91d137c7b2
Offline
#14 2020-08-20 14:47:40
Re: [Discussion] Close the TCrtSocket in case recv() return 0
BTW the same issue is with TrySndLow.
It's not critical BUT - sometimes client (browser in my case) closes connection without waiting for response (I think it's happens than user reload page and some request is in pending state).
In this case I got exceptions in log (raised inside TCrtSocket.SndLow).
But I do not see a how to gracefully sovle it (without braking changes).
The only possible improvement is to exit ASAP if sent=0 (but exception remains)
Offline
#15 2020-08-21 06:39:22
Re: [Discussion] Close the TCrtSocket in case recv() return 0
Please check https://synopse.info/fossil/info/967222a7ae
Offline