Multiple shared_ptr storing same pointer

Question

Consider this program:

#include <memory>
#include <iostream>

class X
  : public std::enable_shared_from_this<X>
{
public:
  struct Cleanup1 { void operator()(X*) const; };
  struct Cleanup2 { void operator()(X*) const; };
  std::shared_ptr<X> lock1();
  std::shared_ptr<X> lock2();
};

std::shared_ptr<X> X::lock1()
{
  std::cout << "Resource 1 locked" << std::endl;
  return std::shared_ptr<X>(this, Cleanup1());
}

std::shared_ptr<X> X::lock2()
{
  std::cout << "Resource 2 locked" << std::endl;
  return std::shared_ptr<X>(this, Cleanup2());
}

void X::Cleanup1::operator()(X*) const
{
  std::cout << "Resource 1 unlocked" << std::endl;
}

void X::Cleanup2::operator()(X*) const
{
  std::cout << "Resource 2 unlocked" << std::endl;
}

int main()
{
  std::cout << std::boolalpha;

  X x;
  std::shared_ptr<X> p1 = x.lock1();
  {
    std::shared_ptr<X> p2 = x.lock2();
  }
}

I don't see anything in the C++11 Standard section 20.7.2 suggesting any of this is invalid. It's a bit unusual to have two shared_ptr objects store the same pointer &x but not share ownership, and to use "deleters" that do not end the lifetime of *get(), but nothing forbids it. (And if either of those are entirely unintended, it would be difficult to explain why some shared_ptr member functions accept a std::nullptr_t value.) And as expected, the program outputs:

Resource 1 locked
Resource 2 locked
Resource 2 unlocked
Resource 1 unlocked

But now if I add a bit to main():

int main()
{
  std::cout << std::boolalpha;

  X x;
  std::shared_ptr<X> p1 = x.lock1();
  bool test1( x.shared_from_this() );
  std::cout << "x.shared_from_this() not empty: " << test1 << std::endl;
  {
    std::shared_ptr<X> p2 = x.lock2();
  }
  try {
    bool test2( x.shared_from_this() );
    std::cout << "x.shared_from_this() not empty: " << test2 << std::endl;
  } catch (std::exception& e) {
    std::cout << "caught: " << e.what() << std::endl;
  }
}

then things get trickier. With g++ 4.6.3, I get the output:

Resource 1 locked
x.shared_from_this() not empty: true
Resource 2 locked
Resource 2 unlocked
caught: std::bad_weak_ptr
Resource 1 unlocked

Why would the second call to shared_from_this() fail? All the requirements of 20.7.2.4p7 are met:

Requires: enable_shared_from_this<T> shall be an accessible base class of T. *this shall be a subobject of an object t of type T. There shall be at least one shared_ptr instance p that owns &t.

[T is X, t is x, p is p1.]

But g++'s enable_shared_from_this essentially follows the suggested implementation from the (non-normative) "Note" in 20.7.2.4p10, using a private weak_ptr member in class enable_shared_from_this. And it seems impossible to account for this sort of issue without doing something considerably more complicated in enable_shared_from_this.

Is this a defect in the Standard? (If so, no comment is needed here on what the solution "should" be: add a requirement so the example program invokes Undefined Behavior, change the Note to not suggest such a simple implementation would be sufficient,....)

Answer 1

Yes, there is a defect here in C++11. In allowing this:

It's a bit unusual to have two shared_ptr objects store the same pointer &x but not share ownership, and to use "deleters" that do not end the lifetime of *get(), but nothing forbids it.

This should be explicitly stated to be undefined behavior, regardless of what the "deleters" do. Sure, it may be technically not illegal to do things that way.

However, you are lying to people who use the code. The expectation of anyone who receives a shared_ptr is that they now have ownership of the object. So long as they keep that shared_ptr (or a copy thereof) around, the object it points to will still exists.

That is not the case with your code. So I would say that it is syntactically correct but semantically invalid.

The language for shared_from_this is fine. It's the language for shared_ptr that needs changing. It should state that it is undefined behavior to create two separate unique pointers that "own" the same pointer.

Answer 2

I agree this is a hole in the specification, thus a defect. It's basically the same as http://open-std.org/jtc1/sc22/wg21/docs/lwg-active.html#2179 although that issue comes at it from a slightly different (and IMHO more obviously broken) angle.

I'm not sure I agree that this is a misuse of shared_ptr, I think it's fine to do that with shared_ptrs, because unlike the code in issue 2179 you use no-op deleters. I think the problem is when you try to combine that kind of use of shared_ptr with enable_shared_from_this.

So my first thought was to fix it by extending the requirements of shared_from_this:

Requires: enable_shared_from_this<T> shall be an accessible base class of T. *this shall be a subobject of an object t of type T. There shall be at least one shared_ptr instance p that owns &t and any other shared_ptr instances that own &t shall share ownership with p.

This isn't quite sufficient though, because your example meets that requirement: at the second call to shared_from_this() there is only one owner (p1) but you've already "corrupted" the state of the enable_shared_from_this base class by calling lock2().

A smaller form of the program is:

#include <memory>
using namespace std;

int main()
{
  struct X : public enable_shared_from_this<X> { };
  auto xraw = new X;
  shared_ptr<X> xp1(xraw);   // #1
  {
    shared_ptr<X> xp2(xraw, [](void*) { });  // #2
  }
  xraw->shared_from_this();  // #3
}

All three of libstdc++, libc++ and VC++ (Dinkumware) behave the same and throw bad_weak_ptr at #3, because at #2 they update the weak_ptr<X> member of the base class to make it share ownership with xp2, which goes out of scope leaving the weak_ptr<X> in the expired state.

Interestingly boost::shared_ptr doesn't throw, instead #2 is a no-op and #3 returns a shared_ptr that shares ownership with xp1. This was done in response to a bug report with almost exactly the same example as the one above.