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

Not all tasks scheduled with Executor are run #171

Closed
ids1024 opened this issue Jan 29, 2024 · 2 comments · Fixed by #172
Closed

Not all tasks scheduled with Executor are run #171

ids1024 opened this issue Jan 29, 2024 · 2 comments · Fixed by #172

Comments

@ids1024
Copy link
Member

ids1024 commented Jan 29, 2024

Not sure exactly what's going on, but some code I have trying to use calloops Executor doesn't always end up running the scheduled tasks. Adding a print to the start of the async { block shows it never starts. Using a different executor doesn't have this problem.

Here's a minimal case where I see an issue like this. It's not really reflective of exactly what I was doing where I ran into this, but it's an interesting case, where "Baz" surely should be printed, but isn't:

fn main() {
    let (executor, scheduler) = calloop::futures::executor().unwrap();
    let scheduler_clone = scheduler.clone();
    scheduler.schedule(async move {
        println!("Foo");
        scheduler_clone.schedule(async {
            println!("Bar");
        });
    });
    let mut event_loop =  calloop::EventLoop::try_new().unwrap();
    event_loop.handle().insert_source(executor, |_: (), _, _| {}).unwrap();
    event_loop.run(None::<std::time::Duration>, &mut (), |_| {
        scheduler.schedule(async {
            println!("Baz");
        });
    });
}
@ids1024
Copy link
Member Author

ids1024 commented Jan 29, 2024

It looks like in both cases, removing if self.notified.swap(true, Ordering::SeqCst) { return; } fixes this. Though that logic seems reasonable. Hm.

@ids1024
Copy link
Member Author

ids1024 commented Jan 29, 2024

Ah, so if schedule is called in the callback invoked by process_events, that will set notified to true (which had been cleared to false at the start of process_events, and send a ping. But the if clear_readiness { ... } block results in that ping being ignored, but doesn't set notified to false.

ids1024 added a commit to ids1024/calloop that referenced this issue Jan 29, 2024
I believe this should be overall more correct. Previously, if the
callback taken by `process_events` scheduled a task, `process_events`
could exit with the readiness of `self.ping` cleared, but `notified` set
to `true`. In which case, future calls to `schedule` would not wake the
event source.

Fixes Smithay#171.
notgull pushed a commit that referenced this issue Jan 30, 2024
I believe this should be overall more correct. Previously, if the
callback taken by `process_events` scheduled a task, `process_events`
could exit with the readiness of `self.ping` cleared, but `notified` set
to `true`. In which case, future calls to `schedule` would not wake the
event source.

Fixes #171.
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