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

Update tokio and use LocalSet for non-Send tasks #274

Merged
merged 4 commits into from Dec 6, 2019

Conversation

swlynch99
Copy link
Contributor

Tokio v0.2.1 adds back the ability to run non-send tasks. This PR removes the hack we had in place to do this and replaces it with LocalSet.

Unfortunately, the LocalSet API currently either panics or hangs when it is dropped with futures that are still live (as would happen when we intercept SIGTERM and try to exit). In cases where these currently panic I've left them alone but I've added one mem::forget call to avoid an infinite loop.

@swlynch99
Copy link
Contributor Author

Going to hold off on merging this until tokio-rs/tokio#1843 is merged and released. After that goes through we can hopefully remove the hacks from here.

Copy link
Contributor

@brayniac brayniac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I'll give an approval after the fix in tokio gets pulled-in.

Copy link
Contributor

@thinkingfish thinkingfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@swlynch99
Copy link
Contributor Author

Currently blocked on tokio-rs/tokio#1885

@hawkw
Copy link

hawkw commented Dec 3, 2019

tokio-rs/tokio#1892 should fix tokio-rs/tokio#1885 (thanks for the report @swlynch99!)

@swlynch99 swlynch99 merged commit 06732f1 into twitter:master Dec 6, 2019
@swlynch99 swlynch99 deleted the update-tokio branch December 6, 2019 21:59
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.

None yet

4 participants