How to implement copyable and movable wrapper around reference counted type?

Question

Suppose a C API provides an opaque struct with internal reference counting:

struct Opaque {
    int data;
    int refcount;
};

struct Opaque* opaque_new(int data) {
    return new Opaque {
        .data = data,
        .refcount = 1
    };
}

int opaque_data(struct Opaque* opaque) {
    return opaque->data;
}

struct Opaque* opaque_ref(struct Opaque* opaque) {
    opaque->refcount++;
    return opaque;
}

void opaque_unref(struct Opaque* opaque) {
    opaque->refcount--;
    if (!opaque->refcount) {
        delete opaque;
    }
}

How to create a wrapper type that is copyable, movable, copy assignable and move assignable?

So far I have:

#include <algorithm>

class Wrapper {
public:
    Wrapper() : m_opaque(nullptr) {}

    explicit Wrapper(int data) : m_opaque(opaque_new(data)) {}

    Wrapper(const Wrapper&) = delete; // TODO

    Wrapper(Wrapper&& wrapper) : m_opaque(wrapper.m_opaque) {
        wrapper.m_opaque = nullptr;
    }

    Wrapper& operator=(const Wrapper&) = delete; // TODO

    Wrapper& operator=(Wrapper&& other) {
        swap(other);
        return *this;
    }

    ~Wrapper() {
        if (m_opaque) {
            opaque_unref(m_opaque);
        }
    }

    void swap(Wrapper& other) {
        std::swap(m_opaque, other.m_opaque);
    }

    int getData() const {
        if (m_opaque) {
            return opaque_data(m_opaque);
        } else {
            return 0;
        }
    }

private:
    struct Opaque* m_opaque;
};

I specifically don't want reference counting on top of reference counting, using std::shared_ptr with a custom deleter.

What's a concise way to implement the remaining methods, in particular copy assignment? Is there a better way to implement the ones I got so far?

Call opaque_ref in any copy operator and save the returned value — gerum, Sep 07 '22 at 15:51
@gerum The copy assignment will also have to destroy the old value. Can I somehow use the destructor for that, similar to the swap trick for move assignment? — Niklas, Sep 07 '22 at 15:55
Sry, I missed that part. If you want to could copy-construct a local variable and then swap the local and this. — gerum, Sep 07 '22 at 15:59
@Niklas you could move ownership of the parameter passed to the copy ctor/assign to a temp variable that only exists for the scope of the ctor/assign. This will automatically call temp's dtor when it leaves scope and destroy the resources accordingly and also have the (maybe, depending on your use case) benefit of leaving the passed parameter in an 'unspecified state` that you could then use later (ie is still 'constructed' but doesn't own data). — oraqlle, Sep 07 '22 at 16:05

score 3 · Accepted Answer · answered Sep 07 '22 at 17:13

Boost's intrusive_ptr was made for "internal reference counts":

#include <boost/intrusive_ptr.hpp>

// intrusive_ptr API functions
inline void intrusive_ptr_add_ref(Opaque* opaque) noexcept {
    ::opaque_ref(opaque);
}
inline void intrusive_ptr_release(Opaque* opaque) noexcept {
    ::opaque_unref(opaque);
}

struct Wrapper {
public:
    // Boost.intrusive_ptr operators are fine
    Wrapper() noexcept = default;
    Wrapper(const Wrapper&) noexcept = default;
    Wrapper(Wrapper&&) noexcept = default;
    Wrapper& operator=(const Wrapper&) noexcept = default;
    Wrapper& operator=(Wrapper&&) noexcept = default;

    // (false means don't add a refcount, since opaque_new does that)
    explicit Wrapper(int data) : m_opaque(opaque_new(data), false) {
        // if (!m_opaque) throw std::bad_alloc();
    }

    void swap(Wrapper& other) noexcept {
        m_opaque.swap(other.m_opaque);
    }
    friend void swap(Wrapper& l, Wrapper& r) noexcept {
        l.swap(r);
    }

    int getData() const {
        if (m_opaque) {
            return ::opaque_data(m_opaque.get());
        } else {
            return 0;
        }
    }
private:
    boost::intrusive_ptr<Opaque> m_opaque;
};

https://godbolt.org/z/oGEdd66Yo

You could also implement this with the "Copy-and-swap" idiom:

Wrapper(Wrapper&& other) noexcept : m_opaque(std::exchange(other.m_opaque, nullptr)) {}

Wrapper(const Wrapper& other) : m_opaque(other.m_opaque) {
    if (m_opaque) opaque_ref(m_opaque);
}

// Also works for move assign
Wrapper& operator=(Wrapper copy) noexcept {
    this->swap(copy);
    return *this;
}

Unrelated, but your C function opaque_new should not throw C++ exceptions (when new fails). You can use new (std::nothrow) Wrapper { ... } so that it returns nullptr instead of throwing an exception. Or you could have it abort instead of throwing.
And would it really hurt to have a nullptr check inside the functions?

oraqlle · Answer 2 · 2022-09-07T16:00:53.897

I'm assuming that you want a discrete copy of the Wrapper object and thus you want m_opaque to point to new data and not the same data (hence using a copy ctor/assignment). In that case you would just create a new pointer to an opaque type copying the values from the opaque type passed to the ctor/assignment-operator.

Note1: for the copy assignment, I added an if that checks that the passed wrap is not 'this' Wrapper (ie w1 = w1). If it is it just returns itself without side effects.

Note2: I also added a getRefCount() method to make it look more seemless. You can make that private/public or whatever or access it directly using ->.

eg.

class Wrapper
{
  /// Other constructors ...

  Wrapper(const Wrapper wrap)
  {
    m_opaque = new Opaque { 
                      .data = wrap.getData(), 
                      .refcount = wrap.getRefCount() 
                   };
  }

  Wrapper& operator=(const Wrapper wrap)
  {
    if (*this != wrap)
    {
      m_opaque = new Opaque { 
                        .data = wrap.getData(), 
                        .refcount = wrap.getRefCount() 
                     };
    }
    
    return *this;
  }

  int getRefCount()
  { return m_opaque->refcount; }

  /// Other methods ...
};

I would assume that he does not want this because if you want to copy the real data also using Wrapper = Opaque would work. — gerum, Sep 07 '22 at 16:00
That's true but from what I got from the question it was `how to make a copy ctor for a wrapper type` ie. a copy ctor for `Wrapper` that makes a copy of a `Wrapper` object. `Wrapper(const Opaque& oq)` is not a copy ctor but a explicit kind of constructor. — oraqlle, Sep 07 '22 at 16:08
How do you get to that converting constructor, nobody talkes about something like that. The interpretation alternative to yours is, to mimic the behaviour of shared_ptr, which has a copy-ctor but it does not copy data. But the final answer what was meant can only come from OP. — gerum, Sep 07 '22 at 18:21

How to implement copyable and movable wrapper around reference counted type?

2 Answers2