YamuxStream is tagged to Unreadable too early, since there may be data that hasn't been processed

[Saigut]

From testing of my program, after YamuxStream tagging stream being unreadable in onFINReceived at this line, there may still be remaining data to be processed in onLengthRead.
So I think tagging stream being unreadable in onFINReceived is too early.

I have no better idea for now, so I just comment this line to workaround this issue.

[creativeid00]

I would say that it is more like the stream acceptance callback is called too late.
I got the same issue during bitswap message receiving from a remote go-ipfs peer. The go-ipfs peers sends 3 frames:

1. frame.type WINDOW_UPDATE (1 '\x1')
   frame.length 0
   frame.flags 1
2. frame.type DATA (0 '\0')
   frame.length 248
   frame.flags 0
3. frame.type WINDOW_UPDATE (1 '\x1')
   frame.length 0
   frame.flags 4
   The last frame contains FIN flag that leads to onFINReceived call. Protocol handler is not called neither after the 1st frame nor after the second one. It is called when the last frame received, some response sent to the remote ipfs peer and Multiselect instance is closed. By that time the yamux stream has already been tagged as Unreadable.

[creativeid00]

Detailed investigation has shown that YamuxStream::doRead returns corresponding error

cpp-libp2p/src/muxer/yamux/yamux_stream.cpp

Line 447 in 91d58f2

return deferReadCallback(Error::STREAM_NOT_READABLE, std::move(read_cb_));

only when no pending data is available in the internal_read_buffer_.

cpp-libp2p/src/muxer/yamux/yamux_stream.cpp

Line 425 in 91d58f2

if (bytes_available_now >= bytes || (some && bytes_available_now > 0)) {

I'm not sure what is a semantics of YamuxStream::isClosedForRead method but it looks like the size of pending data should be checked within the method also.