hashicorp/yamux

stream recvWindow decremented incorretly

apmattil opened this issue · 4 comments

at stream.readData():

       if _, err := io.Copy(s.recvBuf, conn); err != nil {
		s.session.logger.Printf("[ERR] yamux: Failed to read stream data: %v", err)
		s.recvLock.Unlock()
		return err
	}

	// Decrement the receive window
	s.recvWindow -= length <-- this should be number of read bytes.
	s.recvLock.Unlock()

when there is connection breaks io.Copy() will return partial reads.

also unlock missing

if length > s.recvWindow {
		s.session.logger.Printf("[ERR] yamux: receive window exceeded (stream: %d, remain: %d, recv: %d)", s.id, s.recvWindow, length)
                 <--- unlock here -->
		return ErrRecvWindowExceeded
	}

same problem with session.send(), number of copied bytes from io.Copy() is not respected nor returned to e.g stream.Write()

As someone who is not super familiar with the codebase can you explain your third comment? I can see how to fix comment 1 (which is in #107 -- mind a review?) and comment 2 has already been addressed. But it's not clear to me how and where things should be updated on the send side, possibly because it seems like for sending it's done async on a channel so I don't see a very obvious way to get information about what was actually sent.

looks like 3th issue is already fixed.

yamux/stream.go

Line 480 in aad893e

if length > s.recvWindow {

I can not remember how I fixed the send part .. I'll get back about it.