Skip to content
This repository has been archived by the owner on Dec 21, 2020. It is now read-only.

Emit and await signals between greenlets #1

Merged
merged 16 commits into from
Jan 28, 2020
Merged

Emit and await signals between greenlets #1

merged 16 commits into from
Jan 28, 2020

Conversation

rossjrw
Copy link
Owner

@rossjrw rossjrw commented Jan 26, 2020

Continuation of rossjrw/tars#121.

Goal: implementation of Signals. A Signal is an event that is emitted from somewhere, and picked up elsewhere.

Any greenlet can emit a Signal of a given name. Any function that is decorated to indicate that it awaits a signal will be executed in a new greenlet, similar to pyaib's exisitng "watch" method for Events. Additionally, any existing greenlet that is stopped and is waiting for this specific Signal will be resumed.

A Signal can carry data along with it.

@rossjrw
Copy link
Owner Author

rossjrw commented Jan 26, 2020

I'm trying to implement this in roughly the same way that Events are implemented. Resuming stuff seems fine although I haven't tested it yet. Adding a signal waiter as a decorator is actually proving to be more difficult than I expected. Turns out I can't really add a decorator to any old function, it has to be registered with a plugin, which is fair enough. But, a plugin can't have a hook for more than one type of thing. I think the default types are Events and Timers, and now I'm adding Signals. This restriction is explicitly stated but its reason isn't documented, so I'm hoping it won't be an issue.

@rossjrw
Copy link
Owner Author

rossjrw commented Jan 26, 2020

It works!

Problem: a signal, once fired, will always remain fired. What does this actually mean?

pyaib's Events have a single mode of activation: the event happens and any observers (functions that are decorated to indicate they are observing it) are executed with the data from the Event. "Observes" is very apt for Events: these functions are not actively paying attention to the event, they are simply members of a list associated with the Event.

My Signals have two modes of activation. The first is observers, exactly the same as Events. The second are functions that have been paused and are now actively listening for the Signal via gevent events. "Observes" is no longer apt - "watches" is far more appropriate.

When a Signal is fired, two things happen. First, the Signal's internal gevent event is activated. Then, each of the observers are activated. The observers are totally unrelated to the gevent event.

A gevent event stores an internal bool, and firing it consists of setting it to true. In order to be fired again, it needs to be switched back off. Otherwise, when a function pauses to wait for the event, it will immediately resume, because it thinks that event is active.

So there's an inconsistency here: a Signal that hasn't been unfired will work just fine for decorated observers, but paused greenlets will immediately resume. I really truly don't like having two methods in a single thing, but this does seem to be the only way to get both effects that I want.

I'll need to add a function that unfires the signal. Two concerns here:

  • The signal must be unfired every time it's fired.
  • The signal must be unfired only after everything that needs to happen as a result of it has happened.
    The current (not working) function is added as a default observer to every signal, but I don't trust that - I don't know for sure if the waiters for a geven event happen immediately. It might need to be a waiter, but I'm not totally sure how to add that.

I remember reading that gevent event waiters resume in the order that they waited in, so if I add this waiter right before I fire the signal, it should be ok.

@rossjrw
Copy link
Owner Author

rossjrw commented Jan 27, 2020

I remember reading that gevent event waiters resume in the order that they waited in, so if I add this waiter right before I fire the signal, it should be ok.

Changed in version 1.5a3: Waiting greenlets are now awakened in the order in which they waited.

This applies to an alpha version. The current stable version of gevent is 1.4.0.

Force-updating to 1.5a3 doesn't fix this, either.

It occurs to me that I can't actually rely on either the gevent events or the observers to execute last. I have to rely on both. Let's see what happens when I do that.

@rossjrw
Copy link
Owner Author

rossjrw commented Jan 27, 2020

That works, but I still can't confirm whether or not the gevent event actually happens last.

Either way, it's working now, and hasn't yet failed. I'll put this into production to see what happens.

@rossjrw rossjrw marked this pull request as ready for review January 27, 2020 05:57
@rossjrw
Copy link
Owner Author

rossjrw commented Jan 27, 2020

Here's a bug: it works perfectly the first time, then await_signal returns None for all attempts thereafter.

I added a few print statements, which padded things out enough to reveal a race condition.

image

To be clear: the race is between the two waiting greenlets: the one on the bot end, and the internal one that clears the signal.

@rossjrw
Copy link
Owner Author

rossjrw commented Jan 27, 2020

Seems fine now.

@rossjrw
Copy link
Owner Author

rossjrw commented Jan 27, 2020

It's not fine.

Proposing a new solution: instead of unfiring the signal automatically, I could ask the user to unfire it. They'd have to make sure that the signal turns off after it's been used, which could potentially encourage single-thread use only. However, I'd at least be able to guarantee that it's unfired after it's been fired.


Actually, I don't think this will work, either. You can clear the signal every time it's awaited, sure, but it's going to be emitted far more often than that. Either it needs to be cleared after it's emitted instead of after it's awaited - in which case you can't be certain it will reach the waiter before it's unfired - or it needs to be done automatically.

@rossjrw
Copy link
Owner Author

rossjrw commented Jan 27, 2020

Automatic unfiring:

  • The function that unfires is decorated with awaits and also contains an await.
  • The event is set.
  • The function is initiated as an observer. At this point, it is unknown whether or not the waiters on the event have been activated.
  • The function waits for the signal. The waiters may have been activated already, in which case it activates immediately.
    • It's possible that the function may proceed immediately because it detects that the event is set, even though other waiters that have been waiting for longer haven't been activated yet.

@rossjrw
Copy link
Owner Author

rossjrw commented Jan 28, 2020

@jamadden, primary contributor to gevent, suggested a solution (gevent/gevent#1520) that bypasses the unfiring issue completely with a far more sensible method structure.

@rossjrw rossjrw merged commit 979d0e8 into master Jan 28, 2020
@rossjrw rossjrw deleted the signals branch February 14, 2020 17:41
rossjrw added a commit that referenced this pull request Feb 14, 2020
This reverts commit 979d0e8, reversing
changes made to 15075c4.
@rossjrw
Copy link
Owner Author

rossjrw commented Feb 14, 2020

Noting that in order to make a clean pull request to pyaib, I've rebased this branch to remove a commit that fixed a project-specific bug. The "signals" branch has been deleted. The rebased branch is "signal" and is identical to the old branch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant