Skip to content

Remove runtime dependency on async-std by implementing a custom CondVar using the event_listener crate. #4

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

Closed
wants to merge 15 commits into from

Conversation

soruh
Copy link

@soruh soruh commented Jul 13, 2020

This PR completely removes the runtime dependency on async-std as suggested in #3.

My initial approach was to use an async-channels channel but in this case a single CondVar should be sufficient.

The CondVar implementation has only been tested by running the smoke test several times and should be reviewed.

The Atomic accesses can probably all be Relaxed.

This PR does the following:

  • remove the runtime dependency on async-std and add it as a dev-dependency
  • add a dependency on event_listener
  • implement a custom CondVar that is used in place of the async-std channels
  • remove the --features unstable flag on cargo test in CI
  • format the rust source and CI config files (somewhat unintentional so not separated into it's own commit)
  • bump the patch version (it might make sense to bump the minor version and remove the unstable feature)

@soruh
Copy link
Author

soruh commented Jul 13, 2020

I'm reasonably certain that this is safe now.
It's a bit unfortunate that we need have the Event behind what's basically a Box<Arc<Event>> but since we cache the EventListeners we only need to deal with access to that when creating a new StopToken so I think it's okay to do it this way.

@soruh
Copy link
Author

soruh commented Jul 15, 2020

I'm pretty sure this is done.
@matklad it'd be great if you could take a look at this if/when you have the time but no rush.

@yoshuawuyts
Copy link
Collaborator

Thanks for opening this PR! - This feature has been merged as part of #10. Thanks!

@yoshuawuyts yoshuawuyts closed this Oct 2, 2021
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.

3 participants