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

Stacked Borrows: protectors and the dereferenceable attribute #88

Closed
RalfJung opened this issue Feb 7, 2019 · 8 comments
Closed

Stacked Borrows: protectors and the dereferenceable attribute #88

RalfJung opened this issue Feb 7, 2019 · 8 comments
Labels
A-aliasing-model Topic: Related to the aliasing model (e.g. Stacked/Tree Borrows) C-open-question Category: An open question that we should revisit

Comments

@RalfJung
Copy link
Member

RalfJung commented Feb 7, 2019

As tracked in rust-lang/rust#55005, we have a problem with shared references being marked as "dereferencable", in the context of reclamation of memory in concurrent libraries such as Arc.

I originally thought that not pushing protectors for locations inside UnsafeCell fixes this, but it does not: if Arc had a dec method on ArcInner that decrements the refcount, then some protectors would get pushed for it (for the part not inside the UnsafeCell), and get the entire ArcInner could get deallocated while that method is still running. As @nikomatsakis pointed out, you can even imagine a scenario where the refcount is stored out-of-band, and then there is no UnsafeCell in the affected allocation at all.

This can, in principle, also happen in sequential code -- imagine RcInner::dec calling some function after the decrement that might, through an alias, lead to dropping the last remaining aliasing Rc. Or imagine async code and having a yield point after the decrement but before the function returns.

@RalfJung RalfJung added the A-aliasing-model Topic: Related to the aliasing model (e.g. Stacked/Tree Borrows) label Feb 7, 2019
@RalfJung RalfJung added the C-open-question Category: An open question that we should revisit label Aug 14, 2019
@RalfJung RalfJung changed the title Stacked Borrows: barriers and the dereferencable attribute Stacked Borrows: protectors and the dereferencable attribute Sep 2, 2020
@RalfJung
Copy link
Member Author

RalfJung commented Jan 6, 2021

#252 contains a great proposal to tackle this problem -- there are some caveats though.

@RalfJung RalfJung changed the title Stacked Borrows: protectors and the dereferencable attribute Stacked Borrows: protectors and the dereferenceable attribute Jun 20, 2022
@RalfJung
Copy link
Member Author

RalfJung commented Jun 20, 2022

The #252 proposal does not solve the problem for the case "if Arc had a dec method on ArcInner that decrements the refcount".

So maybe it is better to give up on that case, and just go for "not pushing protectors for locations inside UnsafeCell". That has the advantage over #252 that it does not affect &mut.

@RalfJung
Copy link
Member Author

if Arc had a dec method on ArcInner that decrements the refcount, then some protectors would get pushed for it (for the part not inside the UnsafeCell), and get the entire ArcInner could get deallocated while that method is still running. As @nikomatsakis pointed out, you can even imagine a scenario where the refcount is stored out-of-band, and then there is no UnsafeCell in the affected allocation at all.

This is basically exactly what happens here.

@RalfJung
Copy link
Member Author

Doing things specific to the UnsafeCell locations can be rather fragile unless we say that UnsafeCell "infects" the entire reference.

@chorman0773
Copy link
Contributor

chorman0773 commented Jun 13, 2023

Discussed at today's Backlog Bonanza. While the question isn't entirely clear, it seems to be asking specifically about protectors for UnsafeCell, which was fixed by rust#98017. If that's the case, can this issue be closed?

@RalfJung
Copy link
Member Author

RalfJung commented Jun 14, 2023

Well, rust-lang/rust#98017 resolved the concrete problem for Arc, but the entire situation is still not very satisfying. In particular this here demonstrates a major footgun with our current semantics. Specifically, imagine something like

struct S {
  b: AtomicBool,
  i: AtomicUsize,
}

and now we do Arc-like shenanigans and have an &S method that can have the underlying S deallocated during its execution (similar to the "decrement refcount" method). This is still UB! Specifically, the padding bytes between the UnsafeCells in this S are guaranteed to be read-only and "dereferenceable for the entire function call", which is violated if the entire S is deallocated while the method runs.

TB "fixes" that footgun by making UnsafeCell infect the entire reference (Cc #236), but that makes people unhappy for other reasons.

@RalfJung
Copy link
Member Author

Note to self: transition this to be a new issue about tracking the interaction between UnsafeCell and protectors

@RalfJung
Copy link
Member Author

Closing in favor of #433, which describes the current state of the problem more precisely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-aliasing-model Topic: Related to the aliasing model (e.g. Stacked/Tree Borrows) C-open-question Category: An open question that we should revisit
Projects
None yet
Development

No branches or pull requests

2 participants