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

Stuck in waiting for results of AsyncResult #1739

Closed
wjsi opened this issue Jan 5, 2021 · 3 comments
Closed

Stuck in waiting for results of AsyncResult #1739

wjsi opened this issue Jan 5, 2021 · 3 comments

Comments

@wjsi
Copy link

wjsi commented Jan 5, 2021

  • gevent version: 20.12.1 (anaconda)
  • Python version: cPython 3.8.5 (anaconda)
  • Operating System: MacOS 10.15.7

Description:

We need to receive results from an AsyncResult object inside a thread hosted by gevent.threadpool.ThreadPool and it works on gevent<=20.9.0. However, on gevent==20.12.x it stucks.

Adding from gevent import get_hub; get_hub() makes no difference.

What I've run:

Minimal code to reproduce the issue. The code works in 20.9.0 but get stuck in 20.12.0 and 20.12.1.

import gevent.threadpool
import gevent.event


_ar = None


def _waiter(event):
    delay = 0.0005
    while not event.is_set():
        event.wait(delay)
        delay = min(delay * 2, .05)


def gevent_thread():
    global _ar

    local_ev = gevent.event.Event()
    gl = gevent.spawn(_waiter, local_ev)

    _ar = gevent.event.AsyncResult()
    _ar.wait()  # stuck here in gevent==20.12.*
    local_ev.set()
    gl.join()
    print(_ar.get())


def main():
    pool = gevent.threadpool.ThreadPool(1)
    f = pool.spawn(gevent_thread)
    gevent.sleep(0.1)
    _ar.set_result('value')
    f.wait()


if __name__ == '__main__':
    main()
@jamadden
Copy link
Member

jamadden commented Jan 7, 2021

It is not safe, and never has been safe, to use an AsyncResult object from multiple threads. AsyncResult objects are not, and never have been, thread safe (like almost all gevent objects). Depending on a variety of factors, trying to use one from multiple threads could hang, throw errors, or simply result in very undefined behaviour due to race conditions. The exact results have also varied across gevent versions as well.

It may be possible to make this specific example work, but that's due to the very constrained way in which it uses AsyncResult; indeed, this specific example can already be made to work by passing the AsyncResult object from main() into gevent_thread() instead of making gevent_thread allocate it and store it in a global variable, and commenting out the gevent.sleep() in main. At least, it works on my machine with my version of Python right now. But again, AsyncResult is not thread safe and was never intended to be used across threads.

There is an object that can be used to pass results from one thread to another: gevent.threadpool.ThreadResult. This is only semi-public, though: while it has a docstring, it's not documented in the official API docs on gevent.org, and was only intended for the use of the ThreadPool so it isn't especially friendly to use. You could try using that (and I'll consider what it would take to make that a supported, friendly API) but I would recommend trying to recast your problem to avoid needing to use such one-shot objects altogether (e.g., use sequential thread tasks, collecting and passing results from one to the next).

Another thing that may be possible to do here is print an error message. That's tricky, though, because the situation isn't actually unrecoverable, so we can't be sure that the internal issue is actually a hard error.

@jamadden
Copy link
Member

jamadden commented Jan 7, 2021

For reference, what's happening is that _ar.set_result() (usually just spelled _ar.set()...) in the main thread is trying to switch to the greenlet running in the background thread, and that cannot work. (We detect this, and keep the waiting object queued because it is theoretically recoverable: if the background thread used a timed wait and then called wait again, or did so from another greenlet in that thread, everything would work)

gevent is now more careful about which hub gets associated with objects like these, and it detects the fact that it's being used across threads and refuses to try to use a hub for a one thread in a call from a different thread here.

Previously, there was no such check (meaning, operating on an AsyncResult like this was full of race conditions!). It so happened that the way this example was written the main thread was asking the background thread's hub to switch to the background greenlet. This sometimes works but is yet another place that was full of race conditions. Here it happened to work because of the order of operations, and the fact that the example is (kinda) careful to make sure that the _ar.wait() call happens before the _ar.set_result() call.

@wjsi wjsi closed this as completed Jan 11, 2021
@wjsi
Copy link
Author

wjsi commented Jan 11, 2021

Thanks for the instructions. We decide to use other methods to communicate between threads.

jamadden added a commit that referenced this issue Jan 11, 2021
jamadden added a commit that referenced this issue Jan 11, 2021
In addition to the two missing tests mentioned in test__event, I need to see if I can get a semaphore in this situation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants