3

This is more of a follow up question on the second answer posted here. The code from this answer is shown below:

template<typename T>
void do_release(typename boost::shared_ptr<T> const&, T*)
{
}

template<typename T>
typename std::shared_ptr<T> to_std(typename boost::shared_ptr<T> const& p)
{
    return
        std::shared_ptr<T>(
                p.get(),
                boost::bind(&do_release<T>, p, _1));

}

My understanding on above code is, a functor is created from do_release binded with the boost shared_ptr we are trying to convert and is passed in as a custom deleter.

My current thought is (likely wrong): the new standard shared_ptr doesnt hold any existing ref count held by boost shared_ptr but only one ref count for itself after this "conversion". When the standard shared_ptr destructor gets called, it would call the custom deleter which would then trigger the destructor on the boost shared_ptr? So the ref count and life time of the heap resource is still effectively maintained by the boost shared_ptr. What I mean is if the ref count on the boost shared_ptr > 0 (after de-ref by 1) when the custom deleter is called, it would still not destroy the heap memory.

But what if the standard shared_ptr gets copied? Would this conversion still work? I think it would because when the standard share_ptr is copied, it would then increase the ref count, but its the ref count maintained by the standard shared_ptr, so that the overall ref count on the heap resource is still correct. But now the ref count is maintained by both standard + boost shared_ptr?

Am I correct?

Xiyang Liu
  • 81
  • 5
  • 1
    `std::shared_ptr` deleter captures the `boost::shared_ptr`. Once `std::shared_ptr` is destroyed, its deleter too, so it decreases the ref counter of the `boost::shared_ptr`. – Jarod42 Mar 22 '22 at 12:47
  • 2
    Yes, the ref count will be maintained by both. The boost pointer will have 1 extra ref that won't go away until the std pointer has 0 refs – user253751 Mar 22 '22 at 12:49
  • If I'm not mistaken, that deleter could also be simplified (both visually and likely at runtime) by changing `boost::bind(&do_release, p, _1)` to `[p](T*){}`. The function `do_release` could then go away. – Drew Dormann Mar 22 '22 at 13:23
  • Sure would be nice if the boost library would check the C++ version and just use the standard smart pointers built into the language. – Joseph Larson Mar 22 '22 at 14:42

2 Answers2

4

The shared pointer created has a destroy function object (deleter) that has state. In particular it has a copy of the boost shared ptr.

The destruction action does nothing, it is the destruction of deleter that cleans up the boost shared ptr.

There is a problem though:

Does the C++ standard fully specify cleanup of the deleter itself? E.g. it might stay around when only weak references remain? The standard does guarantee a minimum lifetime for the deleter, leaving the possibility that lives longer than that.

Destroying the destruction object is not specified to occur in either the constructor of std::shared_ptr nor the destructor of std::shared_ptr. It would be reasonable to destroy it either after it is called, or when the reference counting block is destroyed - in the second case, we end up with it lasting until the last weak ptr goes away.

From the draft standard:

[Note: It is unspecified whether the pointer remains valid longer than that. This can happen if the implementation doesn’t destroy the deleter until all weak_ptr instances that share ownership with p have been destroyed. — end note]

The possibility to defer destruction of the deleter is clear. So in that case we end up with shared_ptrs that doesn't destruct their element instance when we want it to be called depending on unspecified details of the C++ implementation you run it on. That is a bad idea.

Better Alternative

I would personally go with a similar trick based on the aliasing constructor instead. It is just as simple, has no additional overhead, and the semantics are actually correct:

template<class T>
std::shared_ptr<T>
as_std_shared_ptr(boost::shared_ptr<T> bp)
{
  if (!bp) return nullptr;
  // a std shared pointer to boost shared ptr.  Yes.
  auto pq = std::make_shared<boost::shared_ptr<T>>(std::move(bp));
  // aliasing ctor.  Hide the double shared ptr.  Sneaky.
  return std::shared_ptr<T>(pq, pq.get()->get());
}

This is much less of a hack than the code from the question, because lifetime of the boost shared ptr is no longer tied to the (unspecified) lifetime of the deleter instance.

sehe
  • 374,641
  • 47
  • 450
  • 633
Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • "So with that version, we end up with shared ptrs whose destructor is not called when we want it to be called" - I'd phrase the problem stronger: the "owning deletor" risks making all `weak_ptr` instances behave like `shared_ptr` instances, hence re-introducing the very cycles that `weak_ptr` is primarily suited to prevent, so leading to resource leaks (including potential deadlocks) – sehe Mar 22 '22 at 21:27
  • @sehe But only risks! On some compilers and libraries, it works "just fine". On others, it won't. But yes, the cycles are a big problem. Bigger than I originally thought about. – Yakk - Adam Nevraumont Mar 22 '22 at 21:28
  • The risk is more than enough deterrent. – sehe Mar 22 '22 at 21:31
  • I think you should post this approach on highly popular posts like https://stackoverflow.com/questions/12314967/cohabitation-of-boostshared-ptr-and-stdshared-ptr#comment28669000_12315035 and https://stackoverflow.com/questions/6326757/conversion-from-boostshared-ptr-to-stdshared-ptr. Perhaps some of these should/could be merged. I'd hate for the improved insight to be ignored by people because for years the less accurate answers have been accruing the votes. Ideas on how to best approach this? – sehe Mar 22 '22 at 22:01
  • @sehe the top answers all appear to fix it with a manual reset in operator(). I mean, not as pretty as mine, but they appear to be non-hostile to code bases. – Yakk - Adam Nevraumont Mar 22 '22 at 22:25
  • I think the note applies, in practice, for the optimization of `std::make_shared` with one unique allocation for `T` and the control block. – Jarod42 Mar 23 '22 at 12:47
  • @Jarod42 No, I doubt it. – Yakk - Adam Nevraumont Mar 23 '22 at 13:07
  • @Jarod42 (To be less terse: a shared pointer is a pair of pointers, one to the control block the other to the object. The object can be allocated independently, and if so it has no room for storing a deleter, meanwhile the control block is allocated at the same time as the deleter storage (if any) is needed. And it is natural to either destroy the deleter when the control block object is destroyed, or doing it immediately after it is called, regardless of if make shared was used.) – Yakk - Adam Nevraumont Mar 23 '22 at 15:40
  • Ok, then implementation which would destroy the deleter with the control block would exhibit the problem you expose, whereas the one which destroy the deleter once called would be fine. thanks for clarifications. – Jarod42 Mar 23 '22 at 15:52
2

But what if the standard shared_ptr gets copied? Would this conversion still work? I think it would because when the standard share_ptr is copied

Indeed.

You can just see it for yourself.

CAUTION

The remainder only anecdotally demonstrates behaviour in limited scenarios, on particular implementations.

As @Yakk's answer points out, there is implementation-defined behaviour (unspecified behaviour) will lead to problems. Therefore, I recommend his superior alternative.

I'll use a more modern spelling with lambda instead of bind:

template <typename T> auto to_std(boost::shared_ptr<T> const& p) {
    return std::shared_ptr<T>(p.get(), [p](auto) {});
}

Here's a good demonstration:

Live On Coliru

#include <boost/make_shared.hpp>
#include <boost/shared_ptr.hpp>
#include <iostream>
#include <memory>

template <typename T> auto to_std(boost::shared_ptr<T> const& p) {
    return std::shared_ptr<T>(p.get(), [p](auto) {});
}

struct X {
    ~X() { std::cout << "X::~X()\n"; }
};

int main() {
    auto b = boost::make_shared<X>();
    auto s = to_std(b);

    auto report = [&] {
        std::cout << "boost: " << b.use_count() << "\tstd: " << s.use_count() << "\n";
    };

    report();

    auto b2 = b;
    report();

    std::vector bb(42, b2);
    report();

    auto s2 = s;
    report();

    std::vector ss(17, s2);
    report();

    bb.clear();
    b2.reset();
    report();

    ss.clear();
    report();

    b.reset();
    s.reset();
    report();

    std::cout << "but s2 is still there: " << s2.use_count() << "\n";
    s2.reset();
}

Prints

boost: 2    std: 1
boost: 3    std: 1
boost: 45   std: 1
boost: 45   std: 2
boost: 45   std: 19
boost: 2    std: 19
boost: 2    std: 2
boost: 0    std: 0
but s2 is still there: 1
X::~X()
sehe
  • 374,641
  • 47
  • 450
  • 633
  • From draft standard: util.smartptr.getdeleter/1: [Note: It is unspecified whether the pointer remains valid longer than that. This can happen if the implementation doesn’t destroy the deleter until all weak_ptr instances that share ownership with p have been destroyed. — end note] – Yakk - Adam Nevraumont Mar 22 '22 at 20:56
  • @Yakk-AdamNevraumont Oh. That sounds bad. I guess that's what you meant in your answer as well, but the penny didn't drop. – sehe Mar 22 '22 at 21:28
  • @Yakk-AdamNevraumont Also, I think I misread your take thinking I'd qualify `std::make_shared>(bp)` as considerably **more** of a hack. But now I realize you *hide* the cruft by cleverly applying another layer of indirection using the aliasing constructor (one of my favorite underrated standard library features, as well). So, +100 for this answer. Mind if I attempt some clarifying edit? – sehe Mar 22 '22 at 21:32
  • Sure, go right ahead, this comment is now 15 characters long. – Yakk - Adam Nevraumont Mar 22 '22 at 21:41
  • @Yakk-AdamNevraumont Done, I hope you like the edits. Cheers for teaching me something I didn't think of - not that I ever used this trick in serious situations (but I did temporarily when upgrading legacy code to C++11). Now I'll have to check that the "accepted" answer on the linked source also correctly contains this, because I think I was aware of that answer for some years now. – sehe Mar 22 '22 at 21:54
  • what about using `std::shared_ptr(p.get(), [q = p](auto) mutable { q.reset(); });` so that the ref count is dropped inside the deleter (and thus does not rely on the deleter to destroy the capture)? – OznOg Aug 30 '23 at 08:13