async-lock icon indicating copy to clipboard operation
async-lock copied to clipboard

Fairness of semaphore

Open bikeshedder opened this issue 3 years ago • 3 comments

I was looking for fairness guarantees of the Semaphore implementation and didn't find anything mentioned in the docs. After some digging I found that event_listener::Event is used internally and documented as being fair.

However looking at the implementation of Semaphore::acquire I can see that it is implemented as a loop calling try_acquire and creating a EventListener in order to wait for a permit.

https://github.com/smol-rs/async-lock/blob/48a5b6220e3a0841535959a1f7113e87501dc1ea/src/semaphore.rs#L86-L95

If I'm not mistaken it's possible that this code listens for an event and right after being woken another tasks snatches that permit away. This could lead to starvation where the tasks never manages to get a permit.

I think I've seen that exact behavior in some of my benchmarks where some workers hung forever causing timeouts and outliers.

I was wondering if there is a "easy" and correct way to make this implementation fair.

If that's out of scope for async-lock a small text in the documentation would be nice that it is not designed to be fair.

bikeshedder avatar May 03 '22 23:05 bikeshedder

I was wondering if there is a "easy" and correct way to make this implementation fair.

I wouldn't know of any (at least a simple-enough one). @taiki-e would you have some ideas?

If that's out of scope for async-lock a small text in the documentation would be nice that it is not designed to be fair.

Yeah, we should do that if @taiki-e doesn't have any ideas either.

zeenix avatar May 04 '22 10:05 zeenix

A fair semaphore would also be a requirement to use this in SQLx as otherwise it can easily lead to starvation at high contention. We previously learned this the hard way when we tried to use async-std's MPMC channels as the core primitive for our connection pool: https://github.com/smol-rs/async-channel/issues/6

abonander avatar Jul 19 '22 07:07 abonander

We could implement this in a similar way to Mutex by having a fair flag on the Semaphore. When it's set, it would make users newly acquiring the Semaphore wait for others to take it first.

notgull avatar Dec 31 '22 03:12 notgull