🚀 go-pugleaf

On or about Tue, 4 Mar 2008 21:46:00 -0800 did "Remy Lebeau \(TeamB\)"
<no.spam@no.spam.com> dribble thusly:

>
>"Mike Ruskai" <BUTthannydI@DONTearthlinkLIKE.netSPAM> wrote in message 
>news:ch7ss31acm13alsqj2s3utjhnbusnks1a9@4ax.com...
>
>> ReadBytes() throws an EIdConnClosedGracefully when the socket
>> has been closed by the remote after all data had been sent.
>
>It should not be.  The only time it should be raising that exception is if 
>you are trying to read more than was actually sent.  Otherwise, ReadBytes() 
>should be returning the full requested data without raising an exception, 
>and then the next read/write operation will detect the disconnect 
>afterwards.  Looking at the source, I do not see how that exception could be 
>occuring.
>
>> So, I have to do something like this to read all data:
>
>No, you should not have to do that.  If what you describe is actually 
>happening (which, looking at the soure code, I don't see how it can), then 
>it is a bug that needs fixing.

Sometimes I forget I have code available to check.  I checked the code, and I
see exactly how this is happening.

Here's the relevant bit in IdIOHandler.pas (lines numbered for reference):

01) procedure TIdIOHandler.ReadBytes(var VBuffer: TIdBytes; AByteCount: 
02) Integer; AAppend: Boolean = True);
03) begin
04)   Assert(FInputBuffer<>nil);
05)   if AByteCount > 0 then begin
06)     // Read from stack until we have enough data
07)     while FInputBuffer.Size < AByteCount do begin
08)       ReadFromSource(False);
09)       CheckForDisconnect(True, True); 
10)     end;
11)     FInputBuffer.ExtractToBytes(VBuffer, AByteCount, AAppend);
12)   end else if AByteCount < 0 then begin
13)     ReadFromSource(False, ReadTimeout, False);
14)     CheckForDisconnect(True, True);
15)     FInputBuffer.ExtractToBytes(VBuffer, -1, AAppend);
16)   end;
17) end;

Here's the relevant bit in IdIOHandlerStack.pas:

01) procedure TIdIOHandlerStack.CheckForDisconnect(
02)   ARaiseExceptionIfDisconnected: Boolean; AIgnoreBuffer: Boolean);
03) var
04)   LDisconnected: Boolean;
05) begin
06)   // ClosedGracefully // Server disconnected
07)   // IOHandler = nil // Client disconnected
08)   if ClosedGracefully then begin
09)     if BindingAllocated then begin
10)       Close;
11)       // Call event handlers to inform the user that we were disconnected
12)       DoStatus(hsDisconnected);
13)       //DoOnDisconnected;
14)     end;
15)     LDisconnected := True;
16)   end else begin
17)     LDisconnected := not BindingAllocated;
18)   end;
19)   // Do not raise unless all data has been read by the user
20)   if LDisconnected then begin
21)     if (InputBufferIsEmpty or AIgnoreBuffer) and
ARaiseExceptionIfDisconnected then begin
22)       RaiseConnClosedGracefully;
23)     end;
24)   end;
25) end;

Look at line 9 from the first snippet.  It's calling CheckForDisconnect() with
both arguments True.  The first specifies that an exception should be raised
when the socket is disconnected.  The second specifies that data in the input
buffer should be ignored, which would otherwise prevent the exception from
being raised.

And on line 21 of the second snippet, you see that it's throwing the
exception, despite there being data in the buffer, because of the second
option being True.

The fix would be to change the second argument to CheckForDisconnect() to
False, but I'm going to leave my workaround in place instead, at least until
that bug is fixed in future snapshots.

Perhaps you can add the fix to the codebase, since you're involved with the
project?