Skip to content

Stacked Borrows: protectors and the dereferenceable attribute #88

Closed

Description

@RalfJung

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.

Activity

  1. added
    A-aliasing-modelTopic: Related to the aliasing model (e.g. Stacked/Tree Borrows)
    on Feb 7, 2019
  2. changed the title [-]Stacked Borrows: barriers and the dereferencable attribute[/-] [+]Stacked Borrows: protectors and the dereferencable attribute[/+] on Sep 2, 2020
  3. RalfJung commented on Jan 6, 2021

    @RalfJung
    MemberAuthor

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

  4. changed the title [-]Stacked Borrows: protectors and the dereferencable attribute[/-] [+]Stacked Borrows: protectors and the dereferenceable attribute[/+] on Jun 20, 2022
  5. RalfJung commented on Jun 20, 2022

    @RalfJung
    MemberAuthor

    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.

  6. RalfJung commented on Jun 25, 2022

    @RalfJung
    MemberAuthor

    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.

  7. RalfJung commented on Jun 26, 2022

    @RalfJung
    MemberAuthor

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

  8. chorman0773 commented on Jun 13, 2023

    @chorman0773
    Contributor

    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?

  9. RalfJung commented on Jun 14, 2023

    @RalfJung
    MemberAuthor

    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.

  10. RalfJung commented on Jul 25, 2023

    @RalfJung
    MemberAuthor

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

  11. RalfJung commented on Jul 25, 2023

    @RalfJung
    MemberAuthor

    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

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-aliasing-modelTopic: Related to the aliasing model (e.g. Stacked/Tree Borrows)C-open-questionCategory: An open question that we should revisit

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions