Unsafe code review: semi-owning Weak<RwLock<T>> guard

code review
15

ManuallyDrop vs. MaybeDangling

I just wanted to clarify that ManuallyDrop should have MaybeDangling semantics, since the ManuallyDrop::{drop,take} APIs are very much designed to "consume" the inner value, wherein any references inside it may be dangling after that, whilst still being a part of the ManuallyDrop.

So the fact it currently does not do so is a bug in the language spec semantics. I said so in the issue, but in a rather ill-phrased manner, which seemed to blame miri, the UB-detector tool, for reporting the UB. RalfJ then pointed out that this was not a bug of miri, since it follows the current language spec semantics –even if they're "wrong", may I add–, hence the RFC to offer MaybeDangling (and fix ManuallyDrop).

So until then, we are forced to use MaybeUninit instead, which does require some extra unsafe and whatnot because now we need to pinky-promise our value is not uninit. This is a bit finicky, so it's a good place for a dedicated abstraction. Speaking of which:

Centralizing the MaybeDangling abstraction.

The "lack of MaybeDangling" situation is extremely similar, ecosystem-wise, to the "lack of offset_of! macro" we used to have:

  1. theoretically, the code we have right now is "susceptible to UB in the future", albeit not exploited by current compilers yet;

  2. a proper implementation is underway, but not yet available (so there is no perfect fit yet), but at least it's guaranteed to kick in before the compiler exploits that "future-wise UB";

  3. we have to make do with some workaround, but would like to make the swap to the proper solution the moment it becomes available.

These are the perfect ingredients then for one thing: a dedicated helper crate that centralizes the workaround. In the case of offset_of!, this has been ::memoffset; and now I am thinking we need one such for MaybeDangling:

  • by dedicating a crate to it, the issue is acknowledged:

    • for those unaware of all these "protector shenanigans", much like how, in the past, many people were unaware of &(*dangling_pointer).field being UB, they will have a place for documentation;

    • the RFC/implementation of the proper solution will have something to refer to, and will get extra motivation to get it rolling;

  • for libraries that were hand-rolling their workaround until now:

    • they won't have to be worried about doing it wrong (although in this case using MaybeUninit is rather simple-ish…);

    • the moment the language offers nicer solutions (e.g., fixing the fact ManuallyDrop doesn't have MaybeDangling semantics yet; or actually featuring MaybeDangling), this centralized crate will be the only place needing to be updated to take advantage of it, in a semver-compatible manner (by design of the crate).

      This thus lets downstream users take advantage of the fix without "you", the middle-layer library author, having to lift a finger.

Which is why I'll very soon be releasing a ::maybe-dangling crate for that purpose, which will be featuring:

  • a fixed ManuallyDrop (that is, a MaybeUninit that can assume_init at any point; at least until the language/::core::mem::ManuallyDrop is fixed and then it will just reëxport it);

  • a tentative MaybeDangling (i.e., ManuallyDrop but with Drop back). Alas, in stable Rust, this will require forgoing the <#[may_dangle] T> Drop semantics (the irony is rich).

    • I do have another crate in the works that should offer a stable-Rust polyfill of <#[may_dangle] 'lt> Drop, which will be too cumbersome to be useful i general, but which ought to be a perfect fix for patterns such as ouroboros (wherein the self-referential lifetime needs to have #[may_dangle] semantics). So that may be offered as well, eventually, alongside ManuallyDrop and MaybeDangling

In order to become a trustworthy crate in the ecosystem, it needs to be well-maintained, at least until we get proper MaybeDangling and whatnot in stable Rust. And to be realistic, that can rarely be achieved by a single maintainer (bus factor, etc.), so I'd like to have more owners joining in on this effort: anyone interested?

4 Likes