Simple shared pointer implementation in a single threaded environment (revised)

Question

std::shared_ptr does not stop you from having the same resource managed by multiple control-blocks (and thus independent sets of shared-pointers), even though it is illegal and leads to double-deletes. Additionally, atomic reference-counts don't have any advantage in single-threaded environments but are still more expensive. Thus I wrote the following version to avoid these problems.

(This is a follow-up to: simple shared pointer implementation in single threaded environment)

template <typename T>
struct ThreadUnsafeSharedPtr {
private:
    struct Pair {
        T data;
        size_t cnt;
        template <typename ... Args>
        Pair(Args && ... args): data(std::forward<Args>(args)...), cnt(1) {}
    };
    Pair *ptr;

public:
    template <typename ... Args>
    static ThreadUnsafeSharedPtr make(Args && ... args) {
        ThreadUnsafeSharedPtr p;
        p.ptr = new Pair(std::forward<Args>(args)...);
        return p;
    }

    ThreadUnsafeSharedPtr() : ptr(nullptr) {}

    ThreadUnsafeSharedPtr(const ThreadUnsafeSharedPtr & other) : ptr(other.ptr) {
        if (ptr && &other != this) {
            ++(ptr->cnt);
        }
    }
    ThreadUnsafeSharedPtr & operator=(const ThreadUnsafeSharedPtr & other) {
        if (ptr && &other != this) {
            ptr = other.ptr;
            ++(ptr->cnt);
        }
        return *this;
    }
    ThreadUnsafeSharedPtr(ThreadUnsafeSharedPtr && other) : ptr(other.ptr) {
        if (&other != this) {
            other.ptr = nullptr;
        }
    }
    ThreadUnsafeSharedPtr & operator=(ThreadUnsafeSharedPtr && other) {
        if (&other != this) {
            ptr = other.ptr;
            other.ptr = nullptr;
        }
        return *this;
    }

    ~ThreadUnsafeSharedPtr() {
        if (ptr) {
            if(--ptr->cnt == 0) {
                delete ptr;
            }
        }
    }
    T & operator* () {
        return ptr->data;
    }

    T * operator-> () {
        return &(ptr->data);
    }
};

I have implemented the shared pointer version I talked about. You can find it in codereview.stackexchange.com/q/232130/211223 . To make it optimized for single thread simply chance refcount in resource from atomic to native type. — ALX23z
– ALX23z, Commented Nov 10, 2019 at 2:54

Deduplicator · Accepted Answer · 2019-11-05 15:28:33Z

Unless this is just for learning, always remember that adding yet another smart-pointer will seriously hinder interaction with anyone expecting a different one.

The Design:

The maker-function is mandatory:
1. Cannot use a different allocator.
2. Cannot generally be used to allow sharing across DLL boundaries, as DLLs (in contrast to SOs) don't do symbol unification.
3. Cannot be used with already-allocated resources, or those needing different deallocation.
Your shared-pointers must always point to the full object. Interior pointers are not allowed, nor are base-pointers.
You don't allow for weak pointers at all.
As an aside, the overhead of using atomic reference-counts is small enough that the standard library eschews the complication of adding single-threaded shared-pointers. Consider whether that's really the part you should save on.

The Implementation:

Using in-class-initializers allows you to simplify the ctors.
You could use aggregate-initialization with Pair, no need for a custom ctor.
Explicitly testing for self-assignment is a bad idea, as it pessimises the common case. Anyway, it's completely useless in a ctor!
You should know that -> binds stronger than anything but scope-resolution ::. If you need to, there are many places to look up the operator precedence rules.
Your move-assignment-operator leaks the assignee's state. Fix it by just doing an unconditional swap instead.

πάντα ῥεῖ · Accepted Answer · 2019-11-02 14:51:13Z

Your implementation of the dereferencing operators misses to check for a valid ptr. It should rather look like this:

T & operator* () {
    if(ptr) {
        return ptr->data;
    }
    throw std::runtime_error("Dereferencing invalid ThreadUnsafeSharedPtr.");
}

T * operator-> () {
    if(ptr) {
        return &(ptr->data);
    }
    throw std::runtime_error("Dereferencing invalid ThreadUnsafeSharedPtr.");
}

Also your implementation of copy constructor and assignment don't check if the incoming ThreadUnsafeSharedPtr is pointing to a valid other pointing to data, respectively if ptr is already shared.
I am not sure if you intended to increment the counter for such case.

I'd rather write these like:

ThreadUnsafeSharedPtr(const ThreadUnsafeSharedPtr & other) : ptr(nullptr) {
    if (other.ptr && &other != this) {
        ptr = other.ptr;
        ++(ptr->cnt);
    }
}
ThreadUnsafeSharedPtr & operator=(const ThreadUnsafeSharedPtr & other) {
    if (other.ptr && &other != this) {
        if(ptr && ptr->cnt > 1) { // Counter must be decremented if it is already shared
           --(ptr->cnt);
        }
        ptr = other.ptr;
        ++(ptr->cnt);
    }
    return *this;
}

Generally I'd be cautious with such hand rolled micro optimizations in favor of the usage of standard library classes.

It may be flawed from pitfalls as shown above, and makes your code less portable (e.g. if some stuff should be reused in multithreaded environments).

The overhead introduced with std::shared_ptr is fairly small.

I think checking nullptr in deference function might cause some performance problem, so I intentionally did not do so, BTW I should have provided a operator bool() const method. The rest of your answer is very helpful. — frank
– frank, Commented Nov 2, 2019 at 13:29
@frank Well one could consider to omit that check for release compiling mode (or replace it with an assert()). It makes it unnecessarily hard to debug such errors otherwise. As you're so obsessed with micro-optimizations, are you working in an embedded environment? — πάντα ῥεῖ
– πάντα ῥεῖ, Commented Nov 2, 2019 at 13:35
Micro-optimisation? Hardly. There's a reason those operations and other simple accessors are not checked in any(?) standard library (outside special opt-in debug modes). — Deduplicator
– Deduplicator, Commented Nov 2, 2019 at 14:28

ALX23z · Accepted Answer · 2019-11-04 16:19:45Z

The biggest problem in the design is lack of option for casting and the smart pointer cannot hold properly an interface class. To fix it you need a redesign.

std::shared_ptr deals with it by storing control block separately. It is a control block and not just reference counter, it stores pointer to deleter and has two reference counters for std::weak_ptr support.

Just a few days ago I posted on code review another lightweight version of std::shared_ptr... it has a few issues currently which I'll fix and make another post by the end of the week. You can change it to single-threaded version by simply changing the type of reference counter.

Stack Exchange Network

Simple shared pointer implementation in a single threaded environment (revised)

3 Answers 3

You must log in to answer this question.

Linked

Hot Network Questions

Simple shared pointer implementation in a single threaded environment (revised)

3 Answers 3

You must log in to answer this question.

Linked

Related

Hot Network Questions