Remove Send from ValueDrain iterator

[seanmonstar]

As noted in #355, the ValueDrain iterator has un-synchronized mutation of the extra_values Vec, and using a Drain, a user can get multiple ValueDrains at the same time and use them in scoped threads.

This change removes the Send and Sync traits from ValueDrain. While technically a breaking change, it's expected to not affect anyone, and is the least-bad solution.

[hawkw]

do you think this necessitates an 0.2 release, given

While technically a breaking change, it's expected to not affect anyone, and is the least-bad solution.

[seanmonstar]

The point is to fix technically unsound behavior, without forcing user's to deal with all the other breaking changes.

[hawkw]

yeah, i just wasn't sure if it was a concern that this is technically a semver violation. seems fine to me 👍

[carllerche]

What was the reason to opt for this instead of move values eagerly into the iterator?

[seanmonstar]

Moving them eagerly adds a cost of allocation even if you're not going to use them, just to save a super obscure case.

[carllerche]

@seanmonstar it could be "optimized" to be a small vec, then the allocation is avoided in most cases.

[seanmonstar]

Is all that fiddling worth not making this change? Do we expect even a single person to be sending ValueDrain to another thread?

[carllerche]

Making it !Send will break any usage w/ Tokio & calling .await while holding to the iterator.

[seanmonstar]

Bleck, gross. I've open #362 with a different solution.