moby / vpnkit

A toolkit for embedding VPN capabilities in your application
Apache License 2.0
1.11k stars 190 forks source link

multiplexer: don't fail if Close() is called concurrently to Read(), Write() #632

Closed djs55 closed 1 year ago

djs55 commented 1 year ago

It's legal for a single net.Conn to be shared between goroutines, for example calling Close() on <- ctx.Done() while calls are blocked in Read and Write.

The multiplexer does not impose an ordering in send: each concurrent call simply competes for the writeMutex.

Therefore sequences we considered invalid are actually possible:

Previously we detected these invalid sequences and shutdown the whole multiplexer. The effect would be to completely disable port forwarding.

Instead we drop these out-of-order packets and log (just in case it's interesting). There is a very small risk that an old out-of-order packet could be mixed up with a new packet if the channel ID is reused, but we have a space of 2**32 of those so re-use is unlikely.