Locking a shared_ptr

11
votes

I have an shared object that need to be send to a system API and extract it back later. The system API receives void * only. I cannot use shared_ptr::get() because it do not increases the reference count and it could be released by other threads before extract from the system API. Sending a newed shared_ptr * will work but involves additional heap allocation.

One way to do it is to let the object derived from enable_shared_from_this. However, because this class template owns only a weak_ptr, it is not enough to keep the object from released.

So my solution looks like the following:

class MyClass:public enable_shared_from_this<MyClass> {
private:
    shared_ptr<MyClass> m_this;
public:
    void *lock(){
        m_this=shared_from_this();
        return this;
    }
    static shared_ptr<MyClass> unlock(void *p){
        auto pthis = static_cast<MyClass *>(p);
        return move(pthis->m_this);
    }
/* ... */
}

/* ... */
autp pobj = make_shared<MyObject>(...);
/* ... */
system_api_send_obj(pobj->lock());
/* ... */
auto punlocked = MyClass::unlock(system_api_reveive_obj());

Are there any easier way to do this?

The downside of this solution:

  • it requires an additional shared_ptr<MyClass> in the MyClass object layout, in addition of a weak_ptr in the base class enable_shared_from_this.

  • As I mentioned in the comments, access to lock() and unlock() concurrently is NOT Safe.

  • The worst thing is that this solution can only support lock() once before a call of unlock(). If the same object is to be use for multiple system API calls, additional reference counting must be implemented.

If we have another enable_lockable_shared_from_this class it will be greate:

class MyClass:public enable_lockable_shared_from_this<MyClass> {
/* ... */
}

/* ... */
autp pobj = make_shared<MyObject>(...);
/* ... */
system_api_send_obj(pobj.lock());
/* ... */
auto punlocked = unlock_shared<MyClass>(system_api_reveive_obj());

And the implementation of enable_lockable_shared_from_this is similar as enable_shared_from_this, the only difference is it implements lock() and a helper function unlock_shared. The calling of those functions only explicitly increase and decrease use_count(). This will be the ideal solution because:

  • It eliminate the additional space cost

  • It reuses the facilities existing for shared_ptr to guarantee concurrency safety.

  • The best thing of this solution is that it supports multiple lock() calls seamlessly.

However, the only biggest downside is: it is not available at the moment!

UPDATE:

At least two answers to this question involves a container of pointers. Please compare those solutions with the following:

class MyClass:public enable_shared_from_this<MyClass> {
private:
    shared_ptr<MyClass> m_this;
    mutex this_lock; //not necessory for single threading environment
    int lock_count;
public:
    void *lock(){
        lock_guard lck(this_lock); //not necessory for single threading environment
        if(!lock_count==0)
            m_this=shared_from_this();
        return this;
    }
    static shared_ptr<MyClass> unlock(void *p){
        lock_guard lck(this_lock); //not necessory for single threading environment
        auto pthis = static_cast<MyClass *>(p);
        if(--lock_count>0)
            return pthis->m_this;
        else {
            lock_count=0;
            return move(pthis->m_this); //returns nullptr if not previously locked
        }
    }
/* ... */
}

/* ... */
autp pobj = make_shared<MyObject>(...);
/* ... */
system_api_send_obj(pobj->lock());
/* ... */
auto punlocked = MyClass::unlock(system_api_reveive_obj());

This is absolutely an O(1) vs O(n) (space; time is O(log(n)) or similar, but absolutely greater than O(1) ) game.

4
c++c++11shared-ptr
"Easier way"? Does that solution even work? What good is a MyClass pointer to the API function? Basically, whoever controls the system_api_set_key() and system_api_get_key() should keep a shared pointer around, so that the lifetime of the object is suitably managed. – Kerrek SB
This is working because the object own a strong reference to itself when locked, and this is a cyclic reference until m_this is released. system_api_send_key (sorry I changed the name) and system_api_receive_key is SYSTEM API and I cannot change them. – Earth Engine
The only thing I need to mention is that accessing m_this by multiple thread at the same time is NOT safe. shared_ptr guarantee the multithreading access to the the reference object is safe, but not the shared_ptr itself. So we need some locking surrounding the lock and unlock functions calls. – Earth Engine
Some API enables you to pass a void * as a "handle" that only understandable by the client; the system just store it somewhere and return it in the future. Yes, the code is the same as shared_ptr<MyObject> tmp=m_this; m_this.reset(); return tmp; in C++11. the move constructor guarantee the above. – Earth Engine
@Vaughn : The messages passed through PostMessage and GetMessage are integers specifically sized to be the same size as pointers. I.e., planning to pass a pointer as an integer is perfectly okay in that scenario. – ildjarn

4 Answers

3
votes

I am now have an idea of the following:

template<typename T>
struct locker_helper{
    typedef shared_ptr<T> p_t;
    typedef typename aligned_storage<sizeof(p_t), alignment_of<p_t>::value>::type s_t;
};
template<typename T> void lock_shared(const shared_ptr<T> &p){
    typename locker_helper<T>::s_t value;
    new (&value)shared_ptr<T>(p);
}
template<typename T> void unlock_shared(const shared_ptr<T> &p){
    typename locker_helper<T>::s_t value
        = *reinterpret_cast<const typename locker_helper<T>::s_t *const>(&p);
    reinterpret_cast<shared_ptr<T> *>(&value)->~shared_ptr<T>();
}


template<typename T>
void print_use_count(string s, const shared_ptr<T> &p){
    cout<<s<<p.use_count()<<endl;
}

int main(int argc, char **argv){
    auto pi = make_shared<int>(10);
    auto s = "pi use_count()=";
    print_use_count(s, pi); //pi use_count()=1
    lock_shared(pi);
    print_use_count(s, pi);//pi use_count()=2
    unlock_shared(pi);
    print_use_count(s, pi);//pi use_count()=1
}

and then we can implement the original example as following:

class MyClass:public enable_shared_from_this { /*...*/ };

/* ... */
auto pobj = make_shared<MyClass>(...);
/* ... */
lock_shared(pobj);
system_api_send_obj(pobj.get());
/* ... */
auto preceived = 
    static_cast<MyClass *>(system_api_reveive_obj())->shared_from_this();
unlock_shared(preceived);

It is easy to implement a enable_lockable_shared_from_this with this idea. However, the above is more generic, allows control things that is not derived from enable_lockable_from_this` template class.

3
votes

By adjusting the previous answer, I finally get the following solution:

//A wrapper class allowing you to control the object lifetime
//explicitly.
//
template<typename T> class life_manager{
public:
    //Prevent polymorphic types for object slicing issue.
    //To use with polymorphic class, you need to create
    //a container type for storage, and then use that type.
    static_assert(!std::is_polymorphic<T>::value, 
        "Use on polymorphic types is prohibited.");

    //Example for support of single variable constructor
    //Can be extented to support any number of parameters
    //by using varidict template.
    template<typename V> static void ReConstruct(const T &p, V &&v){ 
        new (const_cast<T *>(&p))T(std::forward<V>(v));
    }

    static void RawCopy(T &target, const T &source){
        *internal_cast(&target) = *internal_cast(&source);
    }
private:
    //The standard said that reterinterpret_cast<T *>(p) is the same as 
    //static_cast<T *>(static_cast<void *>(p)) only when T has standard layout.
    //
    //Unfortunately shared_ptr do not.
    static struct { char _unnamed[sizeof(T)]; } *internal_cast(const T *p){
        typedef decltype(internal_cast(nullptr)) raw;
        return static_cast<raw>(static_cast<void *>(const_cast<T *>(p)));
    }
};

//Construct a new instance of shared_ptr will increase the reference
//count. The original pointer was overridden, so its destructor will
//not run, which keeps the increased reference count after the call.
template<typename T> void lock_shared(const std::shared_ptr<T> &p){
    life_manager<shared_ptr<T> >::ReConstruct(p, std::shared_ptr<T>(p));
}

//RawCopy do bit-by-bit copying, bypassing the copy constructor
//so the reference count will not increase. This function copies
//the shared_ptr to a temporary, and so it will be destructed and
//have the reference count decreased after the call.
template<typename T> void unlock_shared(const std::shared_ptr<T> &p){
    life_manager<shared_ptr<T> >::RawCopy(std::shared_ptr<T>(), p);
}

This is actually the same as my previous version, however. The only thing I did is to create a more generic solution to control object lifetime explicitly.

According to the standard(5.2.9.13), the static_cast sequence is definitely well defined. Furthermore, the "raw" copy behavior could be undefined but we are explicitly requesting to do so, so the user MUST check the system compatibility before using this facility.

Furthermore, actually only the RawCopy() are required in this example. The introduce of ReConstruct is just for general purpose.

2
votes

Why not just wrap the void * API in something that tracks the lifetime of that API's references to your object?

eg.

typedef std::shared_ptr<MyClass> MyPtr;
class APIWrapper
{
    // could have multiple references to the same thing?
    // use multiset instead!
    // are references local to transient messages processed in FIFO order?
    // use a queue!  ... etc. etc.
    std::set<MyPtr, SuitableComparator> references_;

public:
    void send(MyPtr const &ref)
    {
        references_.insert(ref);
        system_api_send_obj(ref.get());
    }
    MyPtr receive(void *api)
    {
        MyPtr ref( static_cast<MyClass *>(api)->shared_from_this() );
        references_.erase(ref);
        return ref;
    }
};

Obviously (hopefully) you know the actual ownership semantics of your API, so the above is just a broad guess.

1
votes

How about using the following global functions that use pointers-to-smart-pointers.

template<typename T> void *shared_lock(std::shared_ptr<T> &sp)
{
    return new std::shared_ptr<T>(sp);
}
template<typename T> std::shared_ptr<T> shared_unlock(void *vp)
{
    std::shared_ptr<T> *psp = static_cast<std::shared_ptr<T,D>*>(sp);
    std::shared_ptr<T> res(*psp);
    delete psp;
    return res;
}

You have an extra new/delete but the original type is unmodified. And additionally, concurrency is not an issue, because each call to shared_lock will return a different shared_ptr.

To avoid the new/delete calls you could use an object pool, but I think that it is not worth the effort.

UPDATE:

If you are not going to use multiple threads in this matter, what about the following?

template<typename T> struct SharedLocker
{
    std::vector< std::shared_ptr<T> > m_ptrs;

    unsigned lock(std::shared_ptr<T> &sp)
    {
        for (unsigned i = 0; i < m_ptrs.size(); ++i)
        {
            if (!m_ptrs[i])
            {
                 m_ptrs[i] = sp;
                 return i;
            }
        }
        m_ptrs.push_back(sp);
        return m_ptrs.size() - 1;
    }
    std::shared_ptr<T> unlock(unsigned i)
    {
        return std::move(m_ptrs[i]);
    }
};

I've changed the void* into an unsigned, but that shouldn't be a problem. You could also use intptr_t, if needed.

Assuming that most of the time there will be only a handful of objects in the vector, maybe even no more than 1, the memory allocations will only happen once, on first insertion. And the rest of the time it will be at zero cost.