rust-lang / rust-lang/rust

`std::ptr::drop_in_place` docs suggest overwriting value with `write`, which can lead to unsoundness in common cases

Open
#127,939 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-docs T-libs
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

Location

std::ptr::drop_in_place

write() can be used to overwrite data without causing it to be dropped

Summary

I was looking for a way to drop a !Unpin future in place in https://github.com/StoicDeveloper/mapped_futures/issues/4 and thought drop_in_place was the obvious choice, and after reading the safety docs, which specifically mention write(), I came up with this inadvertently unsound code:

// Cell is an UnsafeCell<Option<impl !Unpin>> that is possibly Pin, so cannot move
unsafe {
    cell.get().drop_in_place();
    cell.get().write(None);
}

Admittedly, this was my mistake for not reading the example closely, which makes it obvious, but I was probably reading what I wanted to read at the time because drop_in_place was the "obvious" correct choice for dropping a Future in place. But I think if the example wasn't there, it would not be clear to many (most?) readers that this code was unsound from the Safety section alone.

While I think the unsoundness of my code is implied by
While drop_in_place is executing, the only way to access parts of to_drop... and using the pointed-to value after calling... it's unclear that it includes Drop handlers for containers that may attempt to drop value again during unwinding. I think it would help if the docs specifically mentioned panicking and unwinding.

It would also help if it suggested alternate ways to drop and replace a value in place. I would not have guessed *cell.get() = None is the best option (which is used internally in futures-rs). But it's not obvious that that was an option since it looks like it could move the value, where cell.get().drop_in_place() looks more correct because it is explicit about dropping without moving.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the std::ptr::drop_in_place documentation and its Safety section, example, and linked write() documentation. Clarify how unwinding and container Drop handlers affect subsequent access, and document safer ways to drop and replace a value in place. Done means the docs no longer suggest the unsound pattern and clearly explain the relevant alternative.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
documentation
Issue type
Documentation
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.