std::weak_ptr & std::shared_ptr value comparison operators

[sai...@gmail.com]

I'm wondering why these aren't provided, I'm thinking I've missed some kind of issue since it's not present on weak_ptr, and they both to my surprise previously used control block for ordering. Simplified I'm trying to do the following:

std::vector< std::weak_ptr< char > > resVec;

std::shared_ptr< char > newRes(new char, [&](char* res) { resVec.erase(std::find(resVec.begin(), resVec.end(), res )); delete res; });

resVec.push_back(newRes);

This is obviously a no go since the raw pointer can't find the associated weak_ptr. It would seem I'd have to instead use an associative container, even though the weak_ptr already contains the raw pointer.

Kind regards,

Sebastian Karlsson

[Thiago Macieira]

Since the weak_ptrs can asynchronously become disengaged, it's a bad idea to
have comparison operators on them. You should elevate them to shared_ptr
before doing the comparison.

Your case is clearly not affected, since you're comparing a vector of weak_ptr
to a shared_ptr. The problem occurs when you invert it: comparing a vector of
shared_ptr to a weak_ptr: since the weak_ptr may become null half-way through
the loop, the results would depend on timing.

Worse, what happens if someone tried to sort a vector of weak_ptrs? If one of
the elements changes during the procedure, it's quite possible that the
sorting algorithm could crash or enter an infinite loop.

--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center

[sai...@gmail.com]

I don't see how that could happen though since from my understanding the only shared state in regards to the raw pointer is whether or not it's valid or not. I don't see how the value of the local raw pointer can be modified due to lifetime issues of the pointed to object. Sure, as soon as I need to access the pointed to object lock() would be needed to make sure no other thread destroyed it.

Kind regards,

Sebastian Karlsson

[Nicol Bolas]

On Wednesday, June 22, 2016 at 12:53:04 PM UTC-4, sai...@gmail.com wrote:

I don't see how that could happen though since from my understanding the only shared state in regards to the raw pointer is whether or not it's valid or not. I don't see how the value of the local raw pointer can be modified due to lifetime issues of the pointed to object.

If weak_ptrs were comparable, then I would say that logically this should follow: given weak_ptr's wp1 and wp2, the expression `wp1 == wp2` shall evaluate to `true` if, and only if `wp1.lock() == wp2.lock()` would have evaluated to `true`. But that is not necessarily true, since it is very possible for two weak_ptrs to have the same pointer value but with one valid and one invalid:

shared_ptr<K> pk = make_shared<K>(...);
shared_ptr<G> pg = make_shared<G>(...);
Z z;
shared_ptr<Z> pz1 = shared_ptr<Z>(&z, pk);
shared_ptr<Z> pz2 = shared_ptr<Z>(&z, pg);

weak_ptr<Z> wp1 = pz1;
weak_ptr<Z> wp2 = pz2;
pk = nullptr;
pz1 = nullptr;
wp1 == wp2; //true
wp1.lock() == wp2.lock(); //false

The comparison you want would not match the expectations of other users. If you allow comparisons, they will expect that the equivalent `shared_ptr` comparison to hold up.

Similarly, if you have an array of `weak_ptr`s, and you sort it, you would logically expect this array to be in the same order as an array of `shared_ptr` obtained from locking those weak_ptrs. Your comparison will not necessarily allow this either.

If you have some specific need, I would suggest creating your own type that contains a weak_ptr. It'd even be best to store the pointer you're comparing in a `uintptr_t` rather than in some `T*`.

[Thiago Macieira]

On quarta-feira, 22 de junho de 2016 12:11:16 PDT Nicol Bolas wrote:
> shared_ptr<K> pk = make_shared<K>(...);
> shared_ptr<G> pg = make_shared<G>(...);
> Z z;
> shared_ptr<Z> pz1 = shared_ptr<Z>(&z, pk);
> shared_ptr<Z> pz2 = shared_ptr<Z>(&z, pg);

Isn't this here invalid by itself? I would say it has two mistakes:

* you have one object in two different shared_ptr that don't share the
refcounting internals
* you have a shared_ptr of an object that can't be destroyed by delete because
it was allocated on the stack

The latter mistake is unrelated, but it seems to me that the former is quite
valid and the cause of why your comparison fails.

[Nicol Bolas]

On Wednesday, June 22, 2016 at 4:24:12 PM UTC-4, Thiago Macieira wrote:

On quarta-feira, 22 de junho de 2016 12:11:16 PDT Nicol Bolas wrote:
> shared_ptr<K> pk = make_shared<K>(...);
> shared_ptr<G> pg = make_shared<G>(...);
> Z z;
> shared_ptr<Z> pz1 = shared_ptr<Z>(&z, pk);
> shared_ptr<Z> pz2 = shared_ptr<Z>(&z, pg);

Isn't this here invalid by itself?

Reverse the parameters on the shared pointer constructors. My example was meant to invoke the aliasing constructors.
 

I would say it has two mistakes:

* you have one object in two different shared_ptr that don't share the
   refcounting internals
* you have a shared_ptr of an object that can't be destroyed by delete because
   it was allocated on the stack

Given the above correction that aliasing constructors were in use, this should be fine. The aliased pointer will never be destroyed by either `shared_ptr`. They are only storing the pointer, not managing its lifetime.

[Thiago Macieira]

On quarta-feira, 22 de junho de 2016 09:53:04 PDT sai...@gmail.com wrote:
> I don't see how that could happen though since from my understanding the
> only shared state in regards to the raw pointer is whether or not it's
> valid or not. I don't see how the value of the local raw pointer can be
> modified due to lifetime issues of the pointed to object. Sure, as soon as
> I need to access the pointed to object lock() would be needed to make sure
> no other thread destroyed it.

weak_ptr cannot give you the original pointer after the corresponding
shared_ptr has been deleted. It can only give you nullptr after that.

The reason for that is that a pointer that has been freed is not equal to any
valid pointer. It's the same case as:

void f(void *p, void *q)
{
free(p);
return p == q;
}

The compiler is allowed to replace the comparison with "return q;" (check for
null) since a freed pointer is not allowed to be equal to any valid pointer,
so the only condition under which p == q would be if both were nullptr
originally.

With the above in mind, weak_ptr cannot be allowed to return a pointer value
that may be dangling.

PS: I've just noticed that QWeakPointer has operator== and suffers from this
mistake. Fixing it...

[Nevin Liber]

On 22 June 2016 at 11:36, Thiago Macieira <thi...@macieira.org> wrote:

On quarta-feira, 22 de junho de 2016 07:57:31 PDT sai...@gmail.com wrote:
> I'm wondering why these aren't provided, I'm thinking I've missed some kind
> of issue since it's not present on weak_ptr, and they both to my surprise
> previously used control block for ordering. Simplified I'm trying to do the
> following:
>
> std::vector< std::weak_ptr< char > > resVec;
> std::shared_ptr< char > newRes(new char, [&](char* res) {
> resVec.erase(std::find(resVec.begin(), resVec.end(), res )); delete res; });
> resVec.push_back(newRes);
>
> This is obviously a no go since the raw pointer can't find the associated
> weak_ptr. It would seem I'd have to instead use an associative container,
> even though the weak_ptr already contains the raw pointer.

Since the weak_ptrs can asynchronously become disengaged, it's a bad idea to
have comparison operators on them.

That argument doesn't hold for equality comparison.  If the pointers are equal, then you already have a shared_ptr to it (since you are using it for the comparison), and the weak_ptr cannot asynchronously become disengaged.  The only other concern is the atomicity guarantees, but that could be addressed by comparing control block addresses.

Note:  while I believe we could do this for equality comparison, please do not take this as an endorsement that we should do it for equality comparison.  I have not yet seen a motivating case that moves me above "Meh" for this feature.

-- 

 Nevin ":-)" Liber  <mailto:ne...@eviloverlord.com>  +1-847-691-1404

[Nicol Bolas]

On Thursday, June 23, 2016 at 12:44:11 AM UTC-4, Nevin ":-)" Liber wrote:

On 22 June 2016 at 11:36, Thiago Macieira <thi...@macieira.org> wrote:

On quarta-feira, 22 de junho de 2016 07:57:31 PDT sai...@gmail.com wrote:
> I'm wondering why these aren't provided, I'm thinking I've missed some kind
> of issue since it's not present on weak_ptr, and they both to my surprise
> previously used control block for ordering. Simplified I'm trying to do the
> following:
>
> std::vector< std::weak_ptr< char > > resVec;
> std::shared_ptr< char > newRes(new char, [&](char* res) {
> resVec.erase(std::find(resVec.begin(), resVec.end(), res )); delete res; });
> resVec.push_back(newRes);
>
> This is obviously a no go since the raw pointer can't find the associated
> weak_ptr. It would seem I'd have to instead use an associative container,
> even though the weak_ptr already contains the raw pointer.

Since the weak_ptrs can asynchronously become disengaged, it's a bad idea to
have comparison operators on them.

That argument doesn't hold for equality comparison.  If the pointers are equal, then you already have a shared_ptr to it (since you are using it for the comparison), and the weak_ptr cannot asynchronously become disengaged.  The only other concern is the atomicity guarantees, but that could be addressed by comparing control block addresses.

This violates the principle I outlined: comparing control blocks will not yield the same result as comparing the actual pointers. So while doing shared_ptr<T> comparison compares the addresses of the `T`s, doing a weak_ptr<T> comparison would compare the control blocks. So `lhs < rhs` would be different from `lhs.lock() < rhs.lock()`.

[Nevin Liber]

On 23 June 2016 at 09:16, Nicol Bolas <jmck...@gmail.com> wrote:

That argument doesn't hold for equality comparison.  If the pointers are equal, then you already have a shared_ptr to it (since you are using it for the comparison), and the weak_ptr cannot asynchronously become disengaged.  The only other concern is the atomicity guarantees, but that could be addressed by comparing control block addresses.

This violates the principle I outlined: comparing control blocks will not yield the same result as comparing the actual pointers. So while doing shared_ptr<T> comparison compares the addresses of the `T`s, doing a weak_ptr<T> comparison would compare the control blocks. So `lhs < rhs` would be different from `lhs.lock() < rhs.lock()`.

Note: I only talked about equality comparison.

[Thiago Macieira]

On quinta-feira, 23 de junho de 2016 07:16:13 PDT Nicol Bolas wrote:
> This violates the principle I outlined: comparing control blocks will not
> yield the same result as comparing the actual pointers. So while doing
> shared_ptr<T> comparison compares the addresses of the `T`s, doing a
> weak_ptr<T> comparison would compare the control blocks. So `lhs < rhs`
> would be different from `lhs.lock() < rhs.lock()`.

A hypothetical operator< would only need to compare the actual pointers, not
the control blocks. The control blocks can be safely ignored because the
actual pointers must be part of the same data block (or one past the end of
it). Order of pointers that don't belong to the same block is UB.

[Nicol Bolas]

Doesn't matter. Two shared_ptr<T>'s can use the same control block, but refer to different objects through the aliasing constructor. As such, weak_ptr<T>s made from them should likewise compare unequeal.
 

[Nicol Bolas]

On Thursday, June 23, 2016 at 5:01:10 PM UTC-4, Thiago Macieira wrote:

On quinta-feira, 23 de junho de 2016 07:16:13 PDT Nicol Bolas wrote:
> This violates the principle I outlined: comparing control blocks will not
> yield the same result as comparing the actual pointers. So while doing
> shared_ptr<T> comparison compares the addresses of the `T`s, doing a
> weak_ptr<T> comparison would compare the control blocks. So `lhs < rhs`
> would be different from `lhs.lock() < rhs.lock()`.

A hypothetical operator< would only need to compare the actual pointers, not
the control blocks. The control blocks can be safely ignored because the
actual pointers must be part of the same data block (or one past the end of
it). Order of pointers that don't belong to the same block is UB.

Yes, but if the managed object has been destroyed, `weak_ptr<T>::lock` will return a NULL shared_ptr<T>. And that may compare very differently, compared to the pointer value that the `weak_ptr<T>` currently stores.

So long as you hold to the principle that `lhs < rhs` should always be equivalent to `lhs.lock() < rhs.lock()`, then there is no way to compare two `weak_ptr`s. And if you don't hold that principle, then I submit that people would be very surprised indeed by circumstances where it differs.