moby / vpnkit

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

go: add deadlock detector, improve locking #618

Open djs55 opened 1 year ago

djs55 commented 1 year ago

We weren't fully compliant with the net.Conn interface as concurrent Write calls had problems:

  1. concurrent Write calls could exceed the allowed window size: one would see space available, drop the lock to write, allow a second to see the same space and write
  2. concurrent Write calls could block: when the window is closed they would block in Wait() but the window update message only used Signal() and would only wake up one call, leaving the other blocked. It should use Broadcast().

The Write() timeout handling was strange because it offloaded the Wait() to a goroutine. It's simpler to keep the Wait() on the main goroutine and do a Broadcast() from an AfterFunc. It's also good to stop the timer in the common case where you don't need it, rather than leave them to build up.

Also