fix: add read/write locks #85
Conversation
We keep running into races so I figured we might as well just make this all thread-safe. It should have no effect on performance. The actual _bug_ was setting the write deadline while writing. But we might as well just have read and write locks. fixes https://github.com/libp2p/go-libp2p-swarm/issues/205
| // deadline. | ||
|
|
||
| c.writeLock.Lock() | ||
| defer c.writeLock.Unlock() |
There was a problem hiding this comment.
I recommend unifying these into a sync.RWMutex and use lock.RLock() in combination with lock.Lock(). Since the write lock is set without setting the read lock.
There was a problem hiding this comment.
We're locking reads independently from writes. The underlying library does not allow concurrent reads from multiple threads or concurrent writes from multiple threads.
However, reading while writing (as long as only one thread is reading reading and only one thread is writing) is perfectly fine. As a matter of fact, that's how we normally use connections.
Note: the specific bug we were fixing here was concurrently setting a deadline while writing. We could have simply locked the write side and that's it, but we've run into hard to track down race conditions in the past where we were reading from multiple threads without synchronizing (usually in error paths where we're throwing away the connection anyways).
We keep running into races so I figured we might as well just make this all thread-safe. It should have no effect on performance.
The actual bug was setting the write deadline while writing. But we might as well just have read and write locks.
fixes https://github.com/libp2p/go-libp2p-swarm/issues/205