Skip to content
This repository was archived by the owner on Jan 6, 2025. It is now read-only.

Implement next_event_async method polling the event queue #80

Merged
merged 2 commits into from
Jan 5, 2024

Conversation

tnull
Copy link
Collaborator

@tnull tnull commented Jan 3, 2024

Based on #74.

We implement a way to asynchronously poll the queue for new events,
providing an async alternative to wait_next_event.

@tnull tnull force-pushed the 2024-01-async-event-queue branch 4 times, most recently from 40a26e7 to 0bbc34b Compare January 4, 2024 13:50
@@ -65,6 +77,10 @@ impl EventQueue {
drop(queue);

if should_notify {
if let Some(waker) = self.waker.lock().unwrap().take() {
waker.wake();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels a little weird, can you help me understand? is this some weird edge-case where the user is using both the sync wait_next_event and async in tandem somehow / for some reason?

Copy link
Collaborator Author

@tnull tnull Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, kinda. Users likely shouldn't use both at the same time, but here we just make sure to wake the Waker to poll again if we still have events left to be processed, just as we wake up any parked threads via the Condvar. Tbh. I'm not entirely sure this is necessary, but also shouldn't hurt, IIUC.

I now added a test for the event queue operations that has at least some checks for mixing matching blocking/async APIs.

We implement a way to asynchronously poll the queue for new events,
providing an async alternative to `wait_next_event`.
@tnull tnull force-pushed the 2024-01-async-event-queue branch from 0bbc34b to ad59db9 Compare January 5, 2024 09:18
@tnull tnull force-pushed the 2024-01-async-event-queue branch from ad59db9 to 396bdf8 Compare January 5, 2024 09:35
@tnull
Copy link
Collaborator Author

tnull commented Jan 5, 2024

Added a test case for the event queue operations. I'll go ahead and merge this as this only confirms the previously-ACKed code.

@tnull tnull merged commit a514c46 into lightningdevkit:main Jan 5, 2024
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.

2 participants