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

Fix looping in daemon listener loop #244

Merged
merged 1 commit into from
Apr 3, 2023
Merged

Conversation

phil-opp
Copy link
Collaborator

@phil-opp phil-opp commented Apr 3, 2023

The event channel might be closed already inside the loop. When using a normal recv() then, we got a future that immediately resolved to None and let to lots of unnecessary loop iterations, or even endless loops (see #240). This PR fixes this by using a manually constructed future instead, which will remain pending forever once the event channel is closed. This way, we only wait for the next node message in the listener loop.

This is an alternative to #240.

The event channel might be closed already inside the loop. When using a normal `recv()` then, we got a future that immediately resolved to `None` and let to lots of unnecessary loop iterations, or even endless loops. This commit fixes this by using a manually constructed future instead, which will remain pending forever once the event channel is closed. This way, we only wait for the next node message in the listener loop.
@haixuanTao
Copy link
Collaborator

That's a very neat implementation, Thanks!

@haixuanTao haixuanTao enabled auto-merge April 3, 2023 11:19
@haixuanTao haixuanTao merged commit 5686dee into main Apr 3, 2023
@haixuanTao haixuanTao deleted the fix-daemon-listener-loop branch April 3, 2023 11:20
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 this pull request may close these issues.

2 participants