xtaci / kcp-go

A Crypto-Secure Reliable-UDP Library for golang with FEC
MIT License
4.12k stars 737 forks source link

Why UDPSession use mutex? #190

Open small-pineapple opened 4 years ago

small-pineapple commented 4 years ago

It seems UDPSession object can be thread safe, but does it meaningful in golang?

Capybara-1 commented 3 years ago

It seems UDPSession object can be thread safe, but does it meaningful in golang?

Initially I didn't understand why. Kcp directly until I had tested it with kcp.Kcp and when concurrent calls to Recv and Input, there was a data contention problem. His Recv Queue and Send Queue are both a slice, and as you know a slice is not a concurrency-safe data structure. I think that's why UDPSession needs to add a mutually exclusive lock.

small-pineapple commented 3 years ago

I have not read it carefully. However in golang, for one connection(session or something else like that) can have its dedicated goroutine(that's golang one of most important feature). One connection's read write has nothing to do with others. So there is no need to be make synchronization when connections read and write. But KCP does, that seems unnecessary.

Capybara-1 commented 3 years ago

I have not read it carefully. However in golang, for one connection(session or something else like that) can have its dedicated goroutine(that's golang one of most important feature). One connection's read write has nothing to do with others. So there is no need to be make synchronization when connections read and write. But KCP does, that seems unnecessary.

My English is not very good, so I send all the sentences translated through google:.

Because many parts of Kcp are based on Udp cut Kcp will use Input for parsing and Ack reply when receiving Udp message, but Input and Recv will operate the same Kcp structure together, so there will be data competition problem, you can take a look at the source code inside UdpSession, it may give you a good answer.

small-pineapple commented 3 years ago

will operate the same Kcp structure together

If one kcp struct shared with multiple UdpSession, shouldn't Kcp struct has its own mutex to be thread safe? However, the mutex belong to UdpSession, it can not protect the shared kcp structure accessed concurrently, and that aslo shows UdpSession is thread safe object, it methods can be concurrent, but in golang, that was unnessary as I mentioned above

Capybara-1 commented 3 years ago

will operate the same Kcp structure together

If one kcp struct shared with multiple UdpSession, shouldn't Kcp struct has its own mutex to be thread safe? However, the mutex belong to UdpSession, it can not protect the shared kcp structure accessed concurrently, and that aslo shows UdpSession is thread safe object, it methods can be concurrent, but in golang, that was unnessary as I mentioned above

is that each UdpSession holds an exclusive Kcp structure

small-pineapple commented 3 years ago

So the only purpose UdpSession use mutex is make UdpSession can be concurrent. That is somewhat werid, because that should be user's responsibility, for example, make its own session type write func be able to concurrent by wrap a tcpConn with a self defined Session type like:

   UserDefinedSession   (high level, wrap Connection by user, can be thread safety or not according to requirement)

        Connection     (low level, provided by System or lib like tcpConn, udpConn. 
                                Thread safety is not a concern at this level. kcp's thing should
                                at this level at least in user's viewpoint)

This seems a design favor, kcp choose implement its kcp connection be a thread safety object as a plus