[Yamux] Fix sending of buffered messages after a window update

[StefanBratanov]

Mainly changes and refactoring around SendBuffer class. Also using a single windowSizes map for receive and send window sizes since the MuxId is different.

Before logic was checking the total written bytes in the buffer against the sendWindow to determine if it fits within window. That is not entirely correct since the sendWindow is changed inside sendBlocks method. That could lead to incorrect behaviour.

Also there is a possibility that sendWindow is negative, so adding a check which will help with one of the exceptions captured in #311 java.lang.IllegalArgumentException: minimumReadableBytes : -52227 (expected: >= 0)

Also change to calculate totalBufferedWrites on the fly based on the existing SendBuffer values in the map.

Removed data.retain and data.release in SendBuffer.add() and SendBuffer.flush(). Use data.readRetainedSlice() for the partial buffer sending.

[Nashatyrev]

LGTM

nit: TBH I would refactor this part

    private val receiveWindows = ConcurrentHashMap<MuxId, AtomicInteger>()
    private val sendWindows = ConcurrentHashMap<MuxId, AtomicInteger>()
    private val sendBuffers = ConcurrentHashMap<MuxId, SendBuffer>()
    private val totalBufferedWrites = AtomicInteger()

1. Looks like we need just a single Map<MuxId, *>
2. Probably it makes sense to make totalBufferedWrites calculatable on the fly. Seems much safer while probably not that bad with respect to performance

[Nashatyrev]

Right! It was excessive pair of retain/release.
Just to note: the general rule of sending the handler closest to the wire is responsible for releasing the message buffer

[ianopolous]

I think this introduces a vulnerability by merging the send and receive windows. A remote could send a window update for their own receive buffer id, and trick us into setting our receive window for them to a large value?

[Nashatyrev]

@ianopolous thanks for that point 👍
@StefanBratanov what do you think on this?

[StefanBratanov]

@ianopolous thanks for that point 👍 @StefanBratanov what do you think on this?

I think it's a reasonable feedback. Could raise a PR for future discussion.