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

Remove futures-intrusive when using tokio #1833

Closed

Conversation

paolobarbolini
Copy link
Contributor

Currently implemented just for tokio and without waiting for all permits to be released on close

@abonander
Copy link
Collaborator

abonander commented Apr 26, 2022

There unfortunately doesn't seem to be a good alternative for async-std. async-lock doesn't have the API we need, although they're apparently amenable to adding it if you have time to open a PR.

I've already added event-listener to fix #1764 so just using async-lock for all runtimes, assuming it gains the method we need, seems like the easier solution.

@paolobarbolini
Copy link
Contributor Author

paolobarbolini commented May 6, 2022

I've just tested tokio-console and being able to see what the various async resources are doing, including semaphores, is so nice that I'd want to have a different underlying implementation be used depending on the chosen runtime.

@paolobarbolini paolobarbolini force-pushed the byebye-futures-intrusive branch from db844be to 7e5bd66 Compare July 8, 2022 07:31
@paolobarbolini paolobarbolini changed the title Remove futures-intrusive Remove futures-intrusive when using tokio Jul 8, 2022
@paolobarbolini
Copy link
Contributor Author

Would keeping futures-intrusive with async-std sound acceptable? I've implemented that.

@paolobarbolini paolobarbolini marked this pull request as ready for review July 8, 2022 07:32
@abonander
Copy link
Collaborator

That would be acceptable. What's with the cancelled check though?

@abonander
Copy link
Collaborator

Oh, it appears to be hung up, 4h 50m and ticking: https://github.com/launchbadge/sqlx/runs/7257076717?check_suite_focus=true

@paolobarbolini
Copy link
Contributor Author

I'll look into it

@abonander
Copy link
Collaborator

Since it seems like you haven't had the time to come back to this, I'm going to close. Don't worry though, I'm currently doing work involving the runtime features and part of that is using the native runtime types where applicable.

@abonander abonander closed this Aug 9, 2022
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.

2 participants