Skip to content
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

Greenlets that start waiting for an event after it was set awaken before greenlets that started waiting before it was set #1520

Closed
rossjrw opened this issue Jan 27, 2020 · 3 comments · Fixed by #1611

Comments

@rossjrw
Copy link

@rossjrw rossjrw commented Jan 27, 2020

  • gevent version: 1.5a4.dev0
  • Python version: cPython 3.8.0 from Miniconda
  • Operating System: Ubuntu 18.10 (Linux 4.19.84-microsoft-standard under WSL2 on Windows 10 Version 10.0.19041 Build 19041)

Description:

I've created an object that stores a reusable gevent event. Once the event is set, it's critical that it is cleared immediately afterwards - but not before other greenlets that were waiting on it have had a chance to awaken.

1.5a3 states that waiting greenlets awaken in the order in which they waited. The event clearing must happen last, so in order to guarantee that, I've queued it up after the event has been set.

The clearing event happens first, before anything else that started waiting before it. While I admit that it's somewhat illogical for something to start waiting after its awakening condition is true, this is clearly inconsistent behaviour.

What I've run:

Here I've defined one instance of a class Signal, which carries a reusable gevent event and some data that's associated with it. Its intent is to allow for broadcasting data between greenlets in my programme.

Because these signals can be reused, it's important that their internal gevent events are cleared as soon as possible.

import gevent
import gevent.event

class Signal:
    def __init__(self):
        self._event = gevent.event.Event()
        self._data = None

    def fire(self, data):
        self._data = data
        self._event.set()
        self.unfire()

    def unfire(self):
        print("Waiting to unfire")
        await_signal()
        print("Unfiring")
        self._event.clear()
        self._data = None

data_signal = Signal()

def emit_signal(data=None):
    data_signal.fire(data)

def await_signal(timeout=None):
    if not data_signal._event.wait(timeout):
        raise TimeoutError
    return data_signal._data

def start_multiple_greenlets():
    gevent.spawn(signal_waiter)
    gevent.spawn(signal_sender)
    gevent.idle()

def signal_sender(data="data string"):
    print("Sending signal with data={}".format(data))
    emit_signal(data=data)

def signal_waiter(timeout=5.0):
    data = await_signal(timeout=timeout)
    print("Signal recieved with data={}".format(data))

for i in range(10):
    start_multiple_greenlets()

As of 1.5a3, the output data is as follows:

Sending signal with data=data string
Waiting to unfire
Unfiring
Signal recieved with data=None

This indicates that the signal was unfired (and the data destroyed) before signal_waiter got a chance to access the data.

But signal_waiter was the very first thing that was called, and was unambiguously called before unfire - and therefore waited before unfire. Therefore, signal_waiter should have awakened before unfire.

I can't imagine this is intentional behaviour.

@jamadden
Copy link
Member

@jamadden jamadden commented Jan 27, 2020

Every greenlet that's waiting will always be awakened when an Event is set. Whether they want to do anything if the event has already been cleared is up to them, but they are able to know both (a) that it was set while they waited, and (b) whether or not it is still set. They can't know if it flip-flopped multiple times during the interval, however, nor can they know that it flipped only once. That's not what an Event does, and #1521 doesn't really change that.

It looks like you're trying to associate some data with a particular instance of triggering a condition. You can do that, but you need more than just a single Event object to do it reliably. Offhand, I would probably introduce a second level of distribution (a Queue or Channel) to reliably track when and what was signaled (depending on the actual use case, of course). I might make modifications like this (leaving the rest of the program unchanged):

import gevent.queue

class Signal(object):
    def __init__(self):
        self.__event = gevent.event.Event()
        self.__waiters = []

    def fire(self, data):
        "Only waiters waiting when the fire() came in see the *data*"
        waiters = list(self.__waiters)
        self.__waiters.clear()
        gevent.spawn(self._notify, waiters, data)

    @staticmethod
    def _notify(waiters, data):
        for q in waiters:
            q.put_nowait(data)

    def wait(self, timeout):
        q = gevent.queue.Channel()
        self.__waiters.append(evt)
        return q.get(timeout)

def await_signal(timeout=None):
    return data_signal.wait(timeout)

Which produces this output

$ python e.py
Sending signal with data=data string
Signal recieved with data=data string
Sending signal with data=data string
Signal recieved with data=data string
Sending signal with data=data string
Signal recieved with data=data string
Sending signal with data=data string
Signal recieved with data=data string
Sending signal with data=data string
Signal recieved with data=data string
Sending signal with data=data string
Signal recieved with data=data string
Sending signal with data=data string
Signal recieved with data=data string
Sending signal with data=data string
Signal recieved with data=data string
Sending signal with data=data string
Signal recieved with data=data string
Sending signal with data=data string
Signal recieved with data=data string

@rossjrw
Copy link
Author

@rossjrw rossjrw commented Jan 28, 2020

Thanks for your response!

Trust me to use the wrong method for my use case. A little further study of gevent would have done me some good - I passed over the queue documentation and decided on the spot "no, this isn't what I need".

Your script works seemingly reliably. I had an edge case in my project where there was a race condition between unfiring the event and awakening the waiter (instead of just failing all the time) that I couldn't reproduce in my example, and this solves that as well. So a heartfelt thank you for that. My problem is solved.

With regard to the issue at hand, I'm not sure I agree with your decision in #1521, if only because it makes this one edge case inaccurate per 1.5a3's documentation. Though as your solution implies, anyone who discovers that edge case is probably doing something unwise.

I'm happy to agree that this is beyond the scope of Event. Again, thank you for your alternate suggestion.

@jamadden
Copy link
Member

@jamadden jamadden commented Jan 28, 2020

The documentation is using "waiting" to mean "those that are actually blocked inside wait()", i.e., callers which would have raised an exception if they passed timeout=0. It doesn't mean "those that call wait() and discover there's nothing to wait for and return immediately" (because they never waited).

But that's a fine distinction written with kernel colored glasses on. So I'll improve the documentation.

And since there were no test failures and it's only fair, I'll look at what it would take to shift newly arrived callers to wait() to the end of the call list. It may or may not be cheaply possible (these are common primitives so cost matters).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants