V1.2.1 之后的版本有bug

f4nff commented 5 years ago

V1.2.1 之后的版本会导致流量大之后整个隧道关闭,

这个项目更新V1.2.1 之后的版本的版本,使用 kcp模式,流量一大,整个隧道全部断开, V1.2.1版本没问题,但是最新发布的两个版本有bug

xtaci commented 5 years ago

请提供如何重现的例子

f4nff commented 5 years ago

起初我还以为 kcp-go有bug,但是测试了几次,发现是 smux V1.2.2 V1.2.3 有问题,

biotooff commented 5 years ago

copy directly into bytes.Buffer without a tmp buffer https://github.com/xtaci/smux/commit/8a71918d00ec9d14824b26de2ebfea34c3b6190c#diff-f2fccfb96064097b73fbe8eeae3703eeR255 written, err = stream.receiveBytes(s.conn, int64(hdr.Length()))

大流量下突然关闭客户端上层链接会导致返回 0，nil。(eg：视频切换时间轴、关闭视频页面) 数据遗留在队列中未被读取，导致下次读取数据头失败。建议判断下written不等于datalen并丢弃数据

f4nff commented 5 years ago

对,就是这样,突然拖动视频,高清一些的,就会触发,

xtaci commented 5 years ago

之前的是ReadFull，不存在残缺数据

xtaci commented 5 years ago

@biotooff 应该不会返回0, nil的情况

xtaci commented 5 years ago

331 332 // CopyN copies n bytes (or until an error) from src to dst. 333 // It returns the number of bytes copied and the earliest 334 // error encountered while copying. 335 // On return, written == n if and only if err == nil. 336 // 337 // If dst implements the ReaderFrom interface, 338 // the copy is implemented using it. 339 func CopyN(dst Writer, src Reader, n int64) (written int64, err error) {

cs8425 commented 5 years ago

我認為是cmdPSH找不到對應的stream 然後沒有把剩餘的Frame資料讀走接著又直接回去讀取hdr造成錯誤(此時讀到的是cmdPSH的資料, 而非下個Frame) 之前的實作是一定會讀取等於hdr.Length()長度的資料再去讀取下個Frame

xtaci commented 5 years ago

我猜的一个可能原因是receiveBytes如果存在下层阻塞，会导致Close和OpenStream阻塞的问题

xtaci commented 5 years ago

@cs8425 对的

xtaci commented 5 years ago

看来中间的buffer不可避免

cs8425 commented 5 years ago

應該是可以避免中間buffer 只是要判斷如果沒有對應的stream 要記得把剩餘資料讀走&丟棄 (不只cmdPSH, 其他cmd理論上也有可能長度不為0..?

biotooff commented 5 years ago

我認為是cmdPSH找不到對應的stream 然後沒有把剩餘的Frame資料讀走接著又直接回去讀取hdr造成錯誤(此時讀到的是cmdPSH的資料, 而非下個Frame) 之前的實作是一定會讀取等於hdr.Length()長度的資料再去讀取下個Frame

确实

xtaci commented 5 years ago

254             case cmdPSH:
255                 var written int64
256                 var err error
257                 s.streamLock.Lock()
258                 if stream, ok := s.streams[sid]; ok {
259                     written, err = stream.receiveBytes(s.conn, int64(hdr.Length()))
260                     atomic.AddInt32(&s.bucket, -int32(written))
261                     stream.notifyReadEvent()
262                 } else { // discard
263                     io.CopyN(ioutil.Discard, s.conn, int64(hdr.Length()))
264                 }
265                 s.streamLock.Unlock()

xtaci commented 5 years ago

这样应该就ok了

xtaci commented 5 years ago

push了，各位测下

biotooff commented 5 years ago

会出现中途读取失败吗？残余数据不一定等于datalen

xtaci commented 5 years ago

如果出现残余数据，说明链接也断开了，这个时候err会触发s.Close

xtaci commented 5 years ago

@f4nff @cs8425

f4nff commented 5 years ago

@xtaci 简单的测试了下,没问题了

xtaci commented 5 years ago

非常感谢各位的参与测试！

xtaci commented 5 years ago

https://github.com/xtaci/kcptun/releases/tag/v20190418

xtaci commented 5 years ago

https://github.com/xtaci/smux/releases/tag/v1.2.4

xtaci / smux

V1.2.1 之后的版本有bug #48