帧解码器这个地方的锁是否多余了

[YanHeDoki]

每个连接在启动的时候会构造一个解码器实例

func newWebsocketConn(server ziface.IServer, conn *websocket.Conn, connID uint64, r *http.Request) ziface.IConnection {
	// ...
	lengthField := server.GetLengthField()
	if lengthField != nil {
		c.frameDecoder = zinterceptor.NewFrameDecoder(*lengthField)
	}
        // ...
	return c
}`

`func NewFrameDecoder(lf ziface.LengthField) ziface.IFrameDecoder {

	frameDecoder := new(FrameDecoder)
       // ...
	return frameDecoder
}

接收到连接发的包之后是循环读取的，那么应该不会出现多线程并发调用同一个解码器的情况，这个锁是不是可以去掉

func (d *FrameDecoder) Decode(buff []byte) [][]byte {
	d.lock.Lock()
	defer d.lock.Unlock()

	d.in = append(d.in, buff...)
	resp := make([][]byte, 0)

	for {
		arr := d.decode(d.in)

		if arr != nil {
			// Indicates that a complete packet has been parsed
			// (证明已经解析出一个完整包)
			resp = append(resp, arr)
			_size := len(arr) + d.InitialBytesToStrip
			if _size > 0 {
				d.in = d.in[_size:]
			}
		} else {
			return resp
		}
	}
}

以及直接 panic 是不是太粗暴了，虽然外层 recover 了 panic 但是会直接断开连接，抛个错误在外层处理是不是好点

func (d *FrameDecoder) failOnNegativeLengthField(in *bytes.Buffer, frameLength int64, lengthFieldEndOffset int) {
	in.Next(lengthFieldEndOffset)
	panic(fmt.Sprintf("negative pre-adjustment length field: %d", frameLength))
}

[aceld]

@YanHeDoki 嗯，看了下代码，确实Decode() 方法，不会出现并发现象，理论上这个互斥锁没有起到作用。

[aceld]

@YanHeDoki

func (d *FrameDecoder) failOnNegativeLengthField(in *bytes.Buffer, frameLength int64, lengthFieldEndOffset int) {
	in.Next(lengthFieldEndOffset)
	panic(fmt.Sprintf("negative pre-adjustment length field: %d", frameLength))
}

这个panic()，具体如何触发的，然后会带来哪些影响，可能得模拟下，如果改成return后，是否可以继续处理接下来的包，然后如何让应用层感知到，也可能需要设计下。

但是上面那个lock() 应该可以直接提PR修复啦，问题不大。

[YanHeDoki]

@YanHeDoki

func (d *FrameDecoder) failOnNegativeLengthField(in *bytes.Buffer, frameLength int64, lengthFieldEndOffset int) {
	in.Next(lengthFieldEndOffset)
	panic(fmt.Sprintf("negative pre-adjustment length field: %d", frameLength))
}

这个panic()，具体如何触发的，然后会带来哪些影响，可能得模拟下，如果改成return后，是否可以继续处理接下来的包，然后如何让应用层感知到，也可能需要设计下。
但是上面那个lock() 应该可以直接提PR修复啦，问题不大。

@aceld panic() 的情况我测试了一下，会直接被上一层的 StartReader() 函数捕获这个异常，然后走连接断开的流程，我不是很清楚后续还能否再继续解析后续的数据包，但是理论上可以在其中插入一个函数处理到底是直接断开连接还是做其他操作。这个地方需要看下原本这么设计的目的。

[aceld]

@YanHeDoki

func (d *FrameDecoder) failOnNegativeLengthField(in *bytes.Buffer, frameLength int64, lengthFieldEndOffset int) {
	in.Next(lengthFieldEndOffset)
	panic(fmt.Sprintf("negative pre-adjustment length field: %d", frameLength))
}

这个panic()，具体如何触发的，然后会带来哪些影响，可能得模拟下，如果改成return后，是否可以继续处理接下来的包，然后如何让应用层感知到，也可能需要设计下。
但是上面那个lock() 应该可以直接提PR修复啦，问题不大。

@aceld panic() 的情况我测试了一下，会直接被上一层的 StartReader() 函数捕获这个异常，然后走连接断开的流程，我不是很清楚后续还能否再继续解析后续的数据包，但是理论上可以在其中插入一个函数处理到底是直接断开连接还是做其他操作。这个地方需要看下原本这么设计的目的。

@YanHeDoki 嗯，其实，就是这个问题，如果插入这个函数，来处理是否断开还是继续。 继续的话，既然decode已经失败了，那么数据流是否还可以继续解析接下来的数据。 之前的设计的目的，我理解也没有考虑这块，所以就是直接panic了，告诉当前的链接就是不可用的，但是这种确实比较暴力。

	// If the data frame length is less than 0, it means it is an error data packet
	// (如果数据帧长度小于0，说明是个错误的数据包)
	if frameLength < 0 {
		// It will skip the number of bytes of this data packet and throw an exception
		// (内部会跳过这个数据包的字节数，并抛异常)
		d.failOnNegativeLengthField(in, frameLength, d.LengthFieldEndOffset)
	}

现在就是，frameLength<0 才会panic。按照理解，如果frameLength小于0，说明是一个错误的数据包，可以讲整个包丢掉，再继续处理下一个包的。

[YanHeDoki]

@YanHeDoki

func (d *FrameDecoder) failOnNegativeLengthField(in *bytes.Buffer, frameLength int64, lengthFieldEndOffset int) {
	in.Next(lengthFieldEndOffset)
	panic(fmt.Sprintf("negative pre-adjustment length field: %d", frameLength))
}

这个panic()，具体如何触发的，然后会带来哪些影响，可能得模拟下，如果改成return后，是否可以继续处理接下来的包，然后如何让应用层感知到，也可能需要设计下。
但是上面那个lock() 应该可以直接提PR修复啦，问题不大。

@aceld panic() 的情况我测试了一下，会直接被上一层的 StartReader() 函数捕获这个异常，然后走连接断开的流程，我不是很清楚后续还能否再继续解析后续的数据包，但是理论上可以在其中插入一个函数处理到底是直接断开连接还是做其他操作。这个地方需要看下原本这么设计的目的。

@YanHeDoki 嗯，其实，就是这个问题，如果插入这个函数，来处理是否断开还是继续。 继续的话，既然decode已经失败了，那么数据流是否还可以继续解析接下来的数据。 之前的设计的目的，我理解也没有考虑这块，所以就是直接panic了，告诉当前的链接就是不可用的，但是这种确实比较暴力。

	// If the data frame length is less than 0, it means it is an error data packet
	// (如果数据帧长度小于0，说明是个错误的数据包)
	if frameLength < 0 {
		// It will skip the number of bytes of this data packet and throw an exception
		// (内部会跳过这个数据包的字节数，并抛异常)
		d.failOnNegativeLengthField(in, frameLength, d.LengthFieldEndOffset)
	}

现在就是，frameLength<0 才会panic。按照理解，如果frameLength小于0，说明是一个错误的数据包，可以讲整个包丢掉，再继续处理下一个包的。

@aceld 是的，如果直接 panic 不需要这个连接了，那么直接 panic 就可以了，也不需要跳过字节数了,不过可以确定是如果是抛出错误的话，可以尝试在外层添加一个可配置函数用来处理连接或者是记录信息。