Giter Club home page Giter Club logo

retain-ptr's People

Contributors

bruxisma avatar flamefire avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

retain-ptr's Issues

Use of CRT prohibits usage of derived classes

Consider the following MWE:

class Foo: public sg14::reference_count<Foo>{};
class Bar: public Foo{};
sg14::retain_ptr<Foo> foo;
sg14::retain_ptr<Bar> bar;

I would expect this code to compile but as Bar does not inherit from reference_count<Bar> compilation fails:

error: no matching function for call to 'sg14::retain_traits::decrement(sg14::retain_ptr::pointer)'

 if (*this) { traits_type::decrement(this->get()); }

The proposal does not seem to explicitly state this (or I missed it) but intrusive pointers can be used for performance reasons too (avoid additional allocations for ref count opposed shared_ptr). An actual use case I have does this via a base class that "has" the reference counter and many classes that inherit from it. You can then pass around the base class pointer and cast to derived when required, OR pass around the derived class pointer.

The issue described here prevents this.

Solution would be to NOT use CRTP. Changes are trivial and I don't see any disadvantages. It even makes the usage easier. Maybe rename sg14::reference_count to sg14::reference_counter or sg14::reference_counted

Trying to put together minimal C++ usage example

Is

#include <iostream>

using std::cout;
using std::endl;

class S
{
public:
    S(int x_) : x(x_) {}
    ~S()
    {
        cout << __PRETTY_FUNCTION__ << ":" << endl;
    }
public:
    int x;
    mutable unsigned int _rc = 1;
};

struct S_traits final
{
    static void increment (S* x) noexcept
    {
        cout << __PRETTY_FUNCTION__ << ":" << endl;
        x->_rc += 1;
    }
    static void decrement (S* x) noexcept
    {
        x->_rc -= 1;
        cout << __PRETTY_FUNCTION__ << ": rc=" << x->_rc << endl;
        if (x->_rc == 0) { delete x; }
    }
};

#include "sg14/memory.hpp"
using SP = sg14::retain_ptr<S, S_traits>;

int main(__attribute__((unused)) int argc,
         __attribute__((unused)) const char * argv[],
         __attribute__((unused)) const char * envp[])
{
    cout << "sizeof(S): " << sizeof(S) << endl;
    cout << "sizeof(SP): " << sizeof(SP) << endl;
    {
        auto rp = SP(new S(42));
    }
    return 0;
}

which prints

sizeof(S): 8
sizeof(SP): 8
static void S_traits::decrement(S*): rc=0
S::~S():

a correct usage?

Cannot implicit convert to a retain_ptr of a base-class

The following code examples illustrates a current severe limitation with retain_ptr

#include <iostream>
#include <memory>
#include "sg14/memory.hpp"

using namespace std;

class Base
{
public:
    Base()
    {
    }
    mutable unsigned int _rc = 1;
};

class Sub : public Base
{
public:
    Sub(int x_)
        : Base(), x(x_)
    {
        cout << __PRETTY_FUNCTION__ << ":" << endl;
    }
    ~Sub() { cout << __PRETTY_FUNCTION__ << ":" << endl; }
public:
    int x;
};

void test_shared()
{
    using SharedBase = std::shared_ptr<const Base>;
    using SharedSub = std::shared_ptr<const Sub>;
    SharedBase base;
    SharedSub sub;
    base = sub;                 // compiles!
}

struct Base_traits final
{
    static void increment(const Base* x) noexcept
    {
        x->_rc += 1;
        cout << __PRETTY_FUNCTION__ << ": rc=" << x->_rc << endl;
    }
    static void decrement(const Base* x) noexcept
    {
        x->_rc -= 1;
        cout << __PRETTY_FUNCTION__ << ": rc=" << x->_rc << endl;
        if (x->_rc == 0)
        {
            delete x;
        }
    }
};

struct Sub_traits final
{
    static void increment(Sub* x) noexcept
    {
        x->_rc += 1;
        cout << __PRETTY_FUNCTION__ << ": rc=" << x->_rc << endl;
    }
    static void decrement(Sub* x) noexcept
    {
        x->_rc -= 1;
        cout << __PRETTY_FUNCTION__ << ": rc=" << x->_rc << endl;
        if (x->_rc == 0)
        {
            delete x;
        }
    }
};

using RetainBase = sg14::retain_ptr<const Base, Base_traits>;
using RetainSub = sg14::retain_ptr<const Sub, Base_traits>;

void test_retain()
{
    cout << "sizeof(Sub): " << sizeof(Sub) << endl;
    cout << "sizeof(RetainSub): " << sizeof(RetainSub) << endl;
    auto sub = RetainSub(new Sub(42));
    auto base = RetainBase(new Base());
    base = sub; // this errors
}

int main(__attribute__((unused)) int argc,
         __attribute__((unused)) const char* argv[],
         __attribute__((unused)) const char* envp[])
{
    test_shared();
    test_retain();
    return 0;
}

not present in std::shared_ptr. Namely the lack of implicit casting from a retain_ptr of an inherited class (Sub), to a retain_ptr of a base-class (Base) of Sub. Which result in the following compiler error

retain_ptr_test.cpp: In function ‘void test_retain()’:
retain_ptr_test.cpp:83:12: error: no match for ‘operator=’ in ‘base = sub’ (operand types are ‘sg14::retain_ptr<const Base, Base_traits>’ and ‘sg14::retain_ptr<const Sub, Base_traits>’)
In file included from retain_ptr_test.cpp:3:
sg14/memory.hpp:200:17: note: candidate: ‘sg14::retain_ptr<T, R>& sg14::retain_ptr<T, R>::operator=(const sg14::retain_ptr<T, R>&) [with T = const Base; R = Base_traits]’
sg14/memory.hpp:200:17: note:   no known conversion for argument 1 from ‘sg14::retain_ptr<const Sub, Base_traits>’ to ‘const sg14::retain_ptr<const Base, Base_traits>&’
sg14/memory.hpp:205:17: note: candidate: ‘sg14::retain_ptr<T, R>& sg14::retain_ptr<T, R>::operator=(sg14::retain_ptr<T, R>&&) [with T = const Base; R = Base_traits]’
sg14/memory.hpp:205:17: note:   no known conversion for argument 1 from ‘sg14::retain_ptr<const Sub, Base_traits>’ to ‘sg14::retain_ptr<const Base, Base_traits>&&’
sg14/memory.hpp:210:17: note: candidate: ‘sg14::retain_ptr<T, R>& sg14::retain_ptr<T, R>::operator=(std::nullptr_t) [with T = const Base; R = Base_traits; std::nullptr_t = std::nullptr_t]’
sg14/memory.hpp:210:17: note:   no known conversion for argument 1 from ‘sg14::retain_ptr<const Sub, Base_traits>’ to ‘std::nullptr_t’

when compiled with gcc-8 and -std=c++17.

How can I change sg14/memory.hpp to allow this assignment to be allowed?

My two current proposals are

    template <class U>
    using enable_if_sub = std::enable_if_t<std::is_base_of_v<T, U>>;

    template <class S, class = enable_if_sub<S>>
    retain_ptr& operator = (retain_ptr<S, R> &&that) {
        if (*this) { traits_type::decrement(this->get()); } // drop
        ptr = that.detach();
        return *this;
    }

or

    template <class S, class = enable_if_sub<S>>
    retain_ptr& operator = (retain_ptr<S, R> &&that) {
        *this = *reinterpret_cast<retain_ptr<T, R> *>(&that);
        return *this;
    }

or


    template <class S, class = enable_if_sub<S>>
    retain_ptr& operator = (retain_ptr<S, R> &&that) {
        reinterpret_cast<retain_ptr<T, R> *>(&that)->swap(*this);
        return *this;
    }

but all three make AddressSanitizer complain as

=================================================================
==4647==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x602000000010 in thread T0:
  object passed to delete has wrong type:
  size of the allocated type:   8 bytes;
  size of the deallocated type: 4 bytes.
    #0 0x563de6218b28 in operator delete(void*, unsigned long) (/home/per/.emacs.d/auto-builds/gcc-8/AddressSanitized-Debug/home/per/Work/justcxx/retain_ptr_test+0xd7b28)
    #1 0x563de6254df4 in Base_traits::decrement(Base const*) /home/per/Work/justcxx/retain_ptr_test.cpp:52
    #2 0x563de6254f2b in sg14::retain_ptr<Base const, Base_traits>::~retain_ptr() sg14/memory.hpp:197
    #3 0x563de62547fc in test_retain() /home/per/Work/justcxx/retain_ptr_test.cpp:83
    #4 0x563de62548cf in main /home/per/Work/justcxx/retain_ptr_test.cpp:92
    #5 0x7f21d42c1b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
    #6 0x563de6149569 in _start (/home/per/.emacs.d/auto-builds/gcc-8/AddressSanitized-Debug/home/per/Work/justcxx/retain_ptr_test+0x8569)

0x602000000010 is located 0 bytes inside of 8-byte region [0x602000000010,0x602000000018)
allocated by thread T0 here:
    #0 0x563de6217570 in operator new(unsigned long) (/home/per/.emacs.d/auto-builds/gcc-8/AddressSanitized-Debug/home/per/Work/justcxx/retain_ptr_test+0xd6570)
    #1 0x563de625478a in test_retain() /home/per/Work/justcxx/retain_ptr_test.cpp:82
    #2 0x563de62548cf in main /home/per/Work/justcxx/retain_ptr_test.cpp:92
    #3 0x7f21d42c1b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)

SUMMARY: AddressSanitizer: new-delete-type-mismatch (/home/per/.emacs.d/auto-builds/gcc-8/AddressSanitized-Debug/home/per/Work/justcxx/retain_ptr_test+0xd7b28) in operator delete(void*, unsigned long)
==4647==HINT: if you don't care about these errors you may set ASAN_OPTIONS=new_delete_type_mismatch=0
==4647==ABORTING

. What am I doing wrong?

Update:

I think I found a solution

https://github.com/nordlow/justcxx/blob/master/sg14/memory.hpp

in use here

https://github.com/nordlow/justcxx/blob/master/retain_ptr_test.cpp

without any complaints from AddressSantitizer.

Missing declarations in "future"s example

  • atomic_reference_counter is missing
  • rethrow_exception is missing
  • promise<T> calls this->state.template emplace but state is of type retain_ptr<shared_state<T>> which doesn't have an emplace method (maybe it is missing in shared_state<T>?)

Then the retain_ptr is constructed by default as retain_ptr<shared_state<T>> state { new shared_state<T>() };. Is there a way to avoid using new here? Not that is bad per se, but I am missing rationale for a lack of make_retain function in the spirit of make_unique/shared.

Why are the default counts `uint_least64_t` ?

The default counts in atomic_reference_count<T> and reference_count<T> are of type uint_least64_t but there is no rationale for this decision in the paper. In particular:

  • Why isn't 32 bit enough? (e.g. why not use uint_least32_t ?),
  • Why isn't it easily customizable? (e.g. atomic_reference_count<T, C = uint_least64_t>?).

Wrong conditions in retain_traits::decrement

In template <class T> struct retain_traits, the decrement functions seem to delete the object in the wrong situations:

  static void decrement (atomic_reference_count<T>* ptr) noexcept {
    ptr->count.fetch_sub(1, std::memory_order_acq_rel);
    if (not use_count(ptr)) { delete static_cast<T*>(ptr); }
}

Suppose you have two retain_ptr smart pointers pointing to the same object and its reference count is 2. Then, both retain_ptr smart pointers are destructed in different threads. In each thread, fetch_sub decrements the reference count, so it goes from 2 to 1 and then to 0. Then in each thread, use_count returns 0 and both threads delete the same object. decrement should instead check what value fetch_sub returned, so that only the thread that changed the reference count to 0 deletes the object.

  static void decrement (reference_count<T>* ptr) noexcept {
    if (ptr->count -= 1) { delete static_cast<T*>(ptr); }
}

This seems to delete the object when the reference count decreases to any nonzero value. It should delete when the reference count decreases to zero. Commit 898a4fd in PR #6 fixes that.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.