search

Azure / azure-sdk-for-rust

This repository is for active development of the *unofficial* Azure SDK for Rust. This repository is *not* supported by the Azure SDK team.
MIT License
692 stars 237 forks source link

Fixing the naive implementation of sleep #1523

Closed msabansal closed 8 months ago

msabansal commented 8 months ago

Address Issue highlighted in https://github.com/Azure/azure-sdk-for-rust/issues/1515 in the naive implementation.

Delegates to tokio's sleep if tokio feature is turned on

demoray commented 8 months ago

Instead of creating the atomic during poll, why not create it when creating the Sleep object?

msabansal commented 8 months ago

Instead of creating the atomic during poll, why not create it when creating the Sleep object?

Can do that and improve. Wanted to see if this approach works. I had the Issue opened to hope and have the discussion there but since it didnt move i thought i would just open a PR. Also initial implementation thought was to launch thread on first poll

demoray commented 8 months ago

Can do that and improve. Wanted to see if this approach works. I had the Issue opened to hope and have the discussion there but since it didnt move i thought i would just open a PR.

Many (most) of us take much of December off, hence the delay. I'm responding via email as I'm away from work.

I would also like to split off the tokio part as it's own off-by-default feature (and PR), and keep this pr to just fixing the bug you're experiencing.

msabansal commented 8 months ago

Can do that and improve. Wanted to see if this approach works. I had the Issue opened to hope and have the discussion there but since it didnt move i thought i would just open a PR. Many (most) of us take much of December off, hence the delay. I'm responding via email as I'm away from work. I would also like to split off the tokio part as it's own off-by-default feature (and PR), and keep this pr to just fixing the bug you're experiencing.

Also out of office but getting to play around with things :). Thanks for replying and reviewing the PR. Will split it in that manner

msabansal commented 8 months ago

Made the tokio sleep alternative opt-in disabled by default. The naive implementation does a let of work in the poll as it needs to grab the wake context to wake the blocked task.