This ended up getting reverted by #1367, which re-introduced an unsound
use of `Pin::new_unchecked`
See my original PR #1374 for the reasoning behind this change.
- Convert MessageBody to accept Pin in poll_next
- add CHANGES and increase versions aligned to semver
- update crates to accomodate MessageBody Pin change
- fix tests and dependencies
* Moved actix-http for actix from actix crate
* remove resolver feature
* renamed actix feature to actor
* fixed doc attr for actors, add documentation
This removes the last uses of unsafe `Pin` functions in actix-web.
This PR adds a `Pin<Box<_>>` wrapper to `DispatcherState::Upgrade`,
`State::ExpectCall`, and `State::ServiceCall`.
The previous uses of the futures `State::ExpectCall` and `State::ServiceCall`
were Undefined Behavior - a future was obtained from `self.expect.call`
or `self.service.call`, pinned on the stack, and then immediately
returned from `handle_request`. The only alternative to using `Box::pin`
would be to refactor `handle_request` to write the futures directly into
their final location, or avoid polling them before they are returned.
The previous use of `DispatcherState::Upgrade` doesn't seem to be
unsound. However, having data pinned inside an enum that we
`std::mem::replace` would require some careful `unsafe` code to ensure
that we never call `std::mem::replace` when the active variant contains
pinned data. By using `Box::pin`, we any possibility of future
refactoring accidentally introducing undefined behavior.
Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
Fixes#1321
A better fix would be to change `MessageBody` to take a `Pin<&mut
Self>`, rather than a `Pin<&mut Self>`. This will avoid requiring the
use of `Box` for all consumers by allowing the caller to determine how
to pin the `MessageBody` implementation (e.g. via stack pinning).
However, doing so is a breaking change that will affect every user of
`MessageBody`. By pinning the inner stream ourselves, we can fix the
undefined behavior without breaking the API.
I've included @sebzim4500's reproduction case as a new test case.
However, due to the nature of undefined behavior, this could pass (and
not segfault) even if underlying issue were to regress.
Unfortunately, until rust-lang/unsafe-code-guidelines#148 is resolved,
it's not even possible to write a Miri test that will pass when the bug
is fixed.
Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
* Enforce safety of `downcast_ref` at compile time.
The safety of `downcast_ref` requires that `__private_get_type_id__` not
be overriden by callers, since the returned `TypeId` is used to check if
the cast is safe. However, all trait methods in Rust are public, so
users can override `__private_get_type_id__` despite it being
`#[doc(hidden)]`.
This commit makes `__private_get_type_id__` return a type with a private
constructor, ensuring that the only possible implementation is the
default implementation. A more detailed explanation is provided in the
comments added to the file.
Note that the standard library was affected by this type of issue with
the `Error::type_id` function: see https://blog.rust-lang.org/2019/05/14/Rust-1.34.2.html#whats-in-1.34.2-stable
Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
* Replace `UnsafeCell` with `Cell` in `DateServiceInner`
This ensures that it's impossible to cause undefined behavior by
accidentally violating Rust's aliasing rules (e.g. passing a closure to
`set_date` which ends up invoking `reset` or `update` on the inner
`DateServiceInner`).
There might be a tiny amount of overhead from copying the `Option<(Date,
Instant)>` rather than taking a reference, but it shouldn't be
measurable.
Since the wrapped type is `Copy`, a `Cell` can be used, avoiding the
runtime overhead of a `RefCell`.
Co-authored-by: Yuki Okushi <huyuumi.dev@gmail.com>
