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

Probe modul ready signal #121

Closed
djchhp opened this issue Mar 11, 2024 · 0 comments · Fixed by #122
Closed

Probe modul ready signal #121

djchhp opened this issue Mar 11, 2024 · 0 comments · Fixed by #122
Assignees

Comments

@djchhp
Copy link
Contributor

djchhp commented Mar 11, 2024

Issue

The probe module does not wait for everest to signal ready.

Details

The probe module used for testing is supposed to wait for everest to signal ready. The pattern is to call:

await proble_module.wait_to_ready()

The pattern seems to work, but results in a warning after the tests:
probe_module.py:119: RuntimeWarning: coroutine 'Event.wait' was never awaited

Analysis

There is a problem on two levels.

  1. The current implementation does not wait for the ready event, but moves the blocking operation to another thread to be non blocking.
  2. Thread safety. Not moving to another thread results in wait_for always waiting until timeout. The reason for this is, that asyncio.Event is not thread safe (https://docs.python.org/3/library/asyncio-sync.html). Since _ready_event.set is called from a different thread than ready_event.wait, wait waits until timeout, since it only learns about being signaled after the timeout.

Proposed solution

Replace asyncio.Event with threading.Event and mimic asyncio.Event behavior by wrapping its wait in a Coroutine.

@djchhp djchhp self-assigned this Mar 11, 2024
@djchhp djchhp linked a pull request Mar 11, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

1 participant