hyman-m/tinyrpc

Are there data races in serverCodec & clientCodec?

Closed this issue · 5 comments

https://github.com/zehuamama/tinyrpc/blob/855d7a9068b6bb5f7300e54e08998af137731bbd/codec/server.go#L19-L29
ReadRequestHeader,ReadRequestBody, WriteResponse can these functions be executed concurrently?
Why just lock "protects seq, pending",There may be data races about s.request and s.serializer.
Looks like client.go have the same problem.

没有问题的,你看看代码

没有问题的,你看看net/rpc代码

根据你的提示,我阅读了一遍源码。在这个demo里,你应该参考的是这个?https://cs.opensource.google/go/go/+/master:src/net/rpc/jsonrpc/server.go 这里不锁server.req是有原因的。然后就看了一下外层的调用逻辑,即根据server的处理逻辑,ReadHeader和ReadBody是同步执行的,这两个函数会和writeResponse并发执行,代码如下:
server的主要逻辑在https://cs.opensource.google/go/go/+/master:src/net/rpc/server.go;l=459;bpv=0;bpt=1 (Line 459)的 func (server *Server) ServeCodec(codec ServerCodec)这个函数里,一个for同步调用server.readRequest(codec)(包括ReadHeader和ReadBody),然后丢给go routine去并发执行service. call(....) 这个call函数会去调writeResponse。然后server会去处理下一个请求了。

总结一下就是"ReadHeader和ReadBody是同步执行的,这两个会和writeResponse并发执行"
然后回到目前这个问题上,下面两个地方可能就有竞争了。。。
https://github.com/zehuamama/tinyrpc/blob/855d7a9068b6bb5f7300e54e08998af137731bbd/codec/server.go#L44
https://github.com/zehuamama/tinyrpc/blob/855d7a9068b6bb5f7300e54e08998af137731bbd/codec/server.go#L115

这里header上锁了,没有出现竞态问题

这里header上锁了,没有出现禁态问题

对的,这里不是数据竞争的问题了,是外层的server逻辑导致,在这把s.request当成全局变量用就不行了。。。这个s.request只能当成ReadHeader和ReadBody处理过程中的临时变量,不然得同步整个ReaderHeader, ReadBody和WriteResponse吧。。

没有问题啊,jsonrpc也是这样处理的,有其他问题提个pr的单元测试