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

embassy-executor: set expires_at to u64::MAX when task exits #3876

Closed
wants to merge 1 commit into from

Conversation

wllenyj
Copy link
Contributor

@wllenyj wllenyj commented Feb 12, 2025

Ensure the task is not woken up by the time driver by setting the expires_at value to u64::MAX. This change prevents potential issues with task scheduling and ensures proper task management.

FIX: #3875

Ensure the task is not woken up by the time driver by setting the
expires_at value to u64::MAX. This change prevents potential issues
with task scheduling and ensures proper task management.

FIX: embassy-rs#3875

Signed-off-by: wanglei52 <[email protected]>
@Dirbaio
Copy link
Member

Dirbaio commented Feb 12, 2025

We can't write expires_at from the executor thread, it's a race condition. it must only be touched by the time driver, which can run on a different core/thread/interrupt.

It is intended behavior that a task can stay in the timer queue after exiting. If the timer expires when the task is not spawned it does nothing, if it expires after the user has re-spawned the task it just causes a spurious wake (spurious wakes are allowed in Rust async). It's not a "leak" because tasks are statically allocated so the memory wouldn't be reclaimed anyway, and it's not keeping the task marked as "used" so it doesn't prevent re-spawning the task.

Have you run into actual issues due to this, or is this a theoretical concern?

@wllenyj
Copy link
Contributor Author

wllenyj commented Feb 12, 2025

@Dirbaio Thanks for your reply. I will close this.
I am also trying to dynamically alloc tasks (#3334). So I want to save some runtime overhead.

@wllenyj wllenyj closed this Feb 12, 2025
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.

Task will leak when use with_timeout
2 participants