TSocket.Send result

[EugeneK 19]

Right now code for TSocket.Send is following
 

{ Return -1 if error, else return number of byte written                    }
function TCustomWSocket.Send(Data : TWSocketData; Len : Integer) : Integer;
begin
    if (FState <> wsConnected) and (FState <> wsSocksConnected) then begin
        WSocket_Synchronized_WSASetLastError(WSAENOTCONN);
        SocketError('Send');
        Result := -1;
        Exit;
    end;

    bAllSent := FALSE;
    if Len <= 0 then
        Result := 0
    else begin
        Result   := Len;
        PutDataInSendBuffer(Data, Len);
    end;

    if bAllSent then
        Exit;

    TryToSend;

 

So actual sending is happens TryToSend and if it fails result is not -1 but number of bytes written, which is confusing, because it implies that Send was successful. Can we change it to something like

if not TryToSend then
  Result := -1;

 

[Angus Robertson 574]

Not looked at the code, but such a change could mean data being sent twice, since it's already in the transmit buffer and will get sent the next time TryToSend is called.

 

We avoid low level changes unless there is a very good reason. 

 

Angus

 

[FPiette 384]

3 hours ago, EugeneK said:

So actual sending is happens TryToSend and if it fails result is not -1 but number of bytes written, which is confusing, because it implies that Send was successful. Can we change it

I think that if you have a need for such change you don't use ICS as you should. Remember it is asynchronous. Send just means that the data is buffered and will be send as soon as possible. It is correct to return the number of bytes successfully written in the send buffer. Written in the send buffer DOES NOT mean it has been sent at the time Send() returns.

 

If you tell us which problem you actually try to solve, then we could probably help you solve it.

 

[EugeneK 19]

19 hours ago, FPiette said:

I think that if you have a need for such change you don't use ICS as you should. Remember it is asynchronous. Send just means that the data is buffered and will be send as soon as possible. It is correct to return the number of bytes successfully written in the send buffer. Written in the send buffer DOES NOT mean it has been sent at the time Send() returns.

 

If you tell us which problem you actually try to solve, then we could probably help you solve it.

 

Success is asynchronous, failure you can see right away if sendto returns error, just trying to make code simpler.

[EugeneK 19]

21 hours ago, Angus Robertson said:

Not looked at the code, but such a change could mean data being sent twice, since it's already in the transmit buffer and will get sent the next time TryToSend is called.

 

We avoid low level changes unless there is a very good reason. 

 

Angus

 

What happens is that client drops connection but on my side I don't know it so State is still wsConnected and initial check in at the start of Send does not return failure, actual call to sendto inside TryToSend returns error and closes session, but Send still returns number of bytes written. I have to call Connect again in this case which clears buffer so I don't see how it will be sent twice. I handle OnSessionClosed right now, just will be more convenient to see it as result of Send right away.

[FPiette 384]

46 minutes ago, EugeneK said:

Success is asynchronous, failure you can see right away if sendto returns error

You are wrong. You cannot see transmission error before the actual communication is done later, potentially long after send() method returned.

[Angus Robertson 574]

If the connection is closed prematurely, you have no idea how much data the remote client successfully read. 

 

So your high level protocol needs some method of confirming how much data was received and what to send.  The FTP and HTTP protocols allow such thing, in different ways.

 

A few months ago, I was debugging the ICS FTP client and server handling resumed transfers of 50GB files over the public internet, which was fun.  They always worked on a LAN, but the FTP control connection was being closed by some horrible router somewhere after an hour or two of inactivity.  But it alll works now.

 

Angus

 

[EugeneK 19]

2 hours ago, FPiette said:

You are wrong. You cannot see transmission error before the actual communication is done later, potentially long after send() method returned.

Why do you check for it then? This is from TryToSend, which is called from Send, this is exactly what happens, maybe it does not happen always, but that does not mean it never happens.           

            Count := RealSend(Data, Len);
            if Count > 0 then begin
                Dec(FBufferedByteCount, Count);
                if FBufferedByteCount < 0 then
                    FBufferedByteCount := 0;
                FWriteCount := FWriteCount + Count;  { V8.30 was in RealSend }
            end;
            if Count = 0 then
                break;  // Closed by remote

            if Count = SOCKET_ERROR then begin

 

Edited June 28, 2023 by EugeneK

[EugeneK 19]

2 hours ago, Angus Robertson said:

If the connection is closed prematurely, you have no idea how much data the remote client successfully read. 

 

So your high level protocol needs some method of confirming how much data was received and what to send.  The FTP and HTTP protocols allow such thing, in different ways.

 

A few months ago, I was debugging the ICS FTP client and server handling resumed transfers of 50GB files over the public internet, which was fun.  They always worked on a LAN, but the FTP control connection was being closed by some horrible router somewhere after an hour or two of inactivity.  But it alll works now.

 

Angus

 

Yes, but it is a different conversation. I'm just saying if you know that something went wrong it is better to return error and let user handle it than return total length of data have or have not been sent, implying a success.

[FPiette 384]

11 hours ago, EugeneK said:

I'm just saying if you know that something went wrong it is better to return error and let user handle it than return total length of data have or have not been sent, implying a success.

What you seems not understand yet is that Send() don't send data! It just put data in the send buffer and let the component send it when the network is able to accept it. Your application code continue to run long before the data is completely sent. That is asynchronous operation.