New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Semaphore does not check and notify callbacks on rawlink() #1287

Closed
danmilon opened this Issue Oct 12, 2018 · 3 comments

Comments

Projects
None yet
3 participants
@danmilon

danmilon commented Oct 12, 2018

  • gevent version: 1.3.6
  • Python version: 3.7.0 from archlinux repos
  • Operating System: Linux 4.18.7-arch1-1-ARCH

Description:

gevent.wait([semaphore]) does not yield when the semaphore is already ready.

This raises that it would block forever

import gevent.monkey
gevent.monkey.patch_all()
import gevent
import gevent.lock

s = gevent.lock.Semaphore(1)
gevent.wait([s])

This works as expected:

s = gevent.lock.Semaphore(1)
s.acquire()
gevent.spawn_later(1, s.release)
gevent.wait([s])

I would expect it to work like gevent.event.Event, which will notify links when a new callback is registered. If you agree that the semaphore should behave similarly, I'm happy to work on the fix.

@jamadden

This comment has been minimized.

Member

jamadden commented Oct 12, 2018

It's not clear to me that it should behave like an Event. A Semaphore is not an event, it's a special-purpose, low-level primitive.

@damz

This comment has been minimized.

damz commented Oct 13, 2018

@jamadden I don't know if the parallel with Event is relevant or not, but Semaphore.rawlink is currently documented as:

Register a callback to call when a counter is more than zero.

In every other cases, calling rawlink on an object where the condition is already true notifies immediately. Not in the case of Semaphore. Which means that you cannot actually reliably wait on a Semaphore.

Our use case here was a process that needs to either acquire a semaphore or wait for a stop event to become set. This is easy to implement as a busy loop like:

while True:
  if sem.acquire(block=False):
    # Do some process
  if ev.is_set():
    break
  gevent.wait([sem, ev], count=1)

Other than the semaphore not unblocking the wait immediately if it ends up being already set when entering the wait.

It is not a hard thing to workaround, but it is definitely unexpected, and inconsistent with other implementations of the wait protocol.

@jamadden

This comment has been minimized.

Member

jamadden commented Oct 13, 2018

@damz Thanks for the explanation, that makes sense!

@jamadden jamadden self-assigned this Oct 18, 2018

jamadden added a commit that referenced this issue Oct 18, 2018

Make Semaphore.rawlink() start notifiers if needed.
Fixes #1287

Share the link protocol between Semaphore/Event/AsyncResult. There's some ickiness here with Cython, this might be better refactored to use composition?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment