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

Fix race condition related bugs #243

Merged
merged 6 commits into from
Mar 22, 2018
Merged

Fix race condition related bugs #243

merged 6 commits into from
Mar 22, 2018

Conversation

carllerche
Copy link
Member

This PR fixes some race conditions.

  • The copied implementation of AtomicTask is fixed (see Bump tokio-sync v0.1.1 #881).
  • Spurious notifications caused by reusing a token do not happen anymore.

The spurious notifications issue because on OS X write cannot be called until kqueue returns a notification.

This mostly pulls in changes from rust-lang/futures-rs#881, but
also updates Registration to be a bit more obvious as to what is going
on.
This patch adds an ABA guard on token values before registering them
with Mio. This allows catching token reuse and avoid the notification.

This is needed for OS X as the notification is used to determine that a
TCP connect has completed. A spurious notification can potentially cause
write failures.
@srijs
Copy link
Contributor

srijs commented Mar 22, 2018

I started using this branch for my hyper PR, however I ran into the following error in Travis:

thread 'tokio-runtime-worker-0' panicked at 'unexpected task state; Complete', /home/travis/.cargo/git/checkouts/tokio-377c595163f99a10/b48148f/tokio-threadpool/src/task.rs:145:18

(build logs are here: https://travis-ci.org/hyperium/hyper/jobs/356745310)

@carllerche
Copy link
Member Author

@srijs Thanks, so the repro is to the test suite on the branch locally?

@carllerche
Copy link
Member Author

Ok, I'm going to merge this PR as it is an improvement over master. I opened #248 to track the new issue.

@carllerche carllerche merged commit 08c21e7 into master Mar 22, 2018
@carllerche carllerche deleted the races branch March 23, 2018 17:57
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