Tracking issue for Weak::into_raw/from_raw & similar

[vorner]

Changing this to a tracking issue for the feature:

• Pull request Weak::into_raw #60766
• ❓ What to do about dangling Weak (created by Weak::new()) and T: ?Sized? These seem to be incompatible.
• Stabilization

The original proposal

Hello

The Arc has the into_raw and from_raw methods. I think it would be technically possible to have the same on Weak. Obviously, as Weak is non-owning, it would be up to the caller to make sure the pointer is not dangling when used.

Would adding these (and maybe as_raw too ‒ one can get a reference out of Arc, but not from Weak) require an RFC, because the handling of these methods might be even a bit more delicate than the ones on Arc, or is this considered small enough for just a pull request & later stabilization?

Motivation

I've written the arc-swap crate that allows to keep an Arc around but make it point to some other object atomically. It uses the mentioned methods. It would make sense to have weak version of this atomic storage too.

Additionally, the as_raw would make it possible to compare if eg an Arc and Weak point to the same object (which would also come handy in some alternative of the Cache that doesn't hold the object alive needlessly).

[Centril]

Would adding these (and maybe as_raw too ‒ one can get a reference out of Arc, but not from Weak) require an RFC, because the handling of these methods might be even a bit more delicate than the ones on Arc, or is this considered small enough for just a pull request & later stabilization?

A PR is sufficient; the motivation can be justified in there.

[cramertj]

@rust-lang/libs is there any opposition to stabilizing at least .as_raw()? It's hard to imagine wanting that function looking any different than it does today, and it's pretty useful (I don't know another way to deduplicate weak pointers to the same object).

[vorner]

When you bring it up, what needs to be done for stabilizing it all? The API follows the one on Arc, so I don't think any of this could look differently.

There's the problem with unsized Ts ‒ currently, they are simply not supported (because of the interaction with dangling & null pointers). But relaxing the restriction later would be backwards compatible change, so I don't think a decision what exactly to do about that has to block stabilization.

[CAD97]

Relaxing the restriction of T to ?Sized I don't think is compatible with the current safety requirements. On top of that, I think that as is currently written, it's self-conflicting.

[...]

It takes ownership of one weak count. In case a null is passed, a dangling Weak is returned.

Safety

The pointer must represent one valid weak count. In other words, it must point to T which is or was managed by an (A)Rc and the weak count of that (A)Rc must not have reached 0. It is allowed for the strong count to be 0.

[...]

The safety requirement requires that the pointer point to a valid ArcInner/RcBox (side note: probably should make those naming consistent). But then it also says that it accepts null? The reason for this is that as_raw/into_raw return null when the strong count is 0 when the weak is fake (self.inner() returns None).

But on top of that, this is wrong (I just realized writing this up)! Because from_raw(into_raw(_)) on a dangling Weak turns it into the singleton dangling Weak, the weak count of the original will never be decreased! The correct behavior is for Weak::into_raw to return the dangling pointer such that Weak::from_raw can recover the original pointer to allocation and decrease the weak count. This then is fully compatible with T: ?Sized as it's doing the exact same as the non-weak version.

As for as_raw... it's "fine" for it to return null as a helpful hint (as dereferencing null is a faster error), but I think it's more important for as_raw(_); forget(_) to be equivalent to into_raw(_).

[vorner]

It's been a while since I wrote it, so I went back to have a look. I think it is slightly different than how you describe it. If I read through it correctly, there are three kinds of weak pointer values:

• Fully valid ones, pointing to RcBox with strong_count() > 0.
• Dead ones, pointing to RcBox, but with strong_count() = 0. They've run the destructor on the T, but the RcBox is still there.
• Dangling ones. These were created with Weak::new(). They don't have any RcBox allocated for them and their pointer is set to a sentinel value (usize::MAX casted to pointer).

The into_raw()/from_raw() return null() only for the last case. There's no weak count to speak of in that case, so nothing is forgotten to be decreased.

The code needs to special-case the sentinel value somehow anyway, it can't just add the offset to it, because it would overflow and that is AFAIK forbidden thing to do with pointers. It could also return the sentinel value (usize::MAX), but null seems to be as good sentinel value as that other one, with the difference users recognize it better. It also has proper alignment for the type, which other code might rely on (by putting flags into the low bits). The downside is that pointers to unsized types don't have null() and if I remember correctly, this was the reason why they are not currently supported.

I can try changing the sentinel value and see if I can add support for unsized types instead.

As for the documentation, if I just remove all notions of null and keep it to the fact the pointer must come from previous Weak and not specify what value is returned in case it is from Weak::new(), would it be better?

[CAD97]

Ah, I admit, I read the code wrong.

Ok, I agree returning null for "fake" weak pointers is probably the correct sentinel. (The use of usize::MAX as a sentinel for the inner pointer only works because the RcBox/ArcInner has align(usize) > 1, but the T doesn't have that guarantee, so usize::MAX is a theoretically valid pointer. I think a T pointer within an RcBox/ArcInner is guaranteed to have at least align(inner) because of the alignment of the container, but it'd be better to not rely on that.)

But the nonexistence of ptr::null for ?Sized types isn't really an issue. The reason it doesn't exist is the inability for it to conjure pointer metadata out of thin air. We have that!

I don't know the exact way std would accomplish this, but "all" that needs to be done is to pull the metadata off of the inner pointer, and put it onto a null pointer. (And then do the same thing in from_raw.)

For docs, I'd just specify that the pointer given to Weak::from_raw must have come from Weak::as_raw or Weak::into_raw.

[vorner]

I've tried updating the docs, I hope it's for the better.

But the nonexistence of ptr::null for ?Sized types isn't really an issue. The reason it doesn't exist is the inability for it to conjure pointer metadata out of thin air. We have that!

I don't know the exact way std would accomplish this, but "all" that needs to be done is to pull the metadata off of the inner pointer, and put it onto a null pointer. (And then do the same thing in from_raw.)

It's not the problem with null pointers per se, because Weak::new exists for Sized types only too. It's simply illegal to put null pointer into into_raw for unsized type for that reason. However the sized implementation needs to check for null equality. It would be fine if this turned out to be false every time for unsized types, but as the null() doesn't even exist, it doesn't compile :-(. I think specialization would help here but AFAIK it's not ready to be used yet. Or is it?

But, after clarifying the documentation, is there anything that would prevent adding that specialization/relaxing the bounds in the future in backwards compatible manner?