Thread: ~shared_ptr() block until shared_ptr::unique is true

I'm writing a "Session" class that represents a publishing/subscribing session with a message bus broker.

I'd like owners of a Session to be able to allocate it however they like, but the Session will also have handles to it held by sections of code that want to publish through it.

Shared ownership of the Session is too strong, I think, since publishers are not guaranteed the ability to publish. We only must make sure that there is no race between the Session being destroyed and any in-progress publishing.

What I'd like to have, then, is something like this:

Code:

class Session {
    shared_ptr<SessionHandle> handle;
  public:
    void publish(T data);
    ~Session() {
        // Have the shared_ptr's destructor block until handle.unique() == true
    }

Code:

struct SessionHandle {
    Session & session;
};

Code:

class PublishHandle {
    weak_ptr<SessionHandle> handle;
  public:
    void publish(T data) {
        shared_ptr<SessionHandle> liveHandle = handle.lock();
        if (!liveHandle)
            throw SomeError();

        liveHandle->session.publish(data);
    }
};

Somebody who wants to publish would then hold a PublishHandle. I'd design applications to be structured in such a way that the Session always outlives the publishers anyway (so a plain reference could do), but this is a good check against violations to that.

I know that shared_ptr isn't designed to do what I'm asking. What is the cleanest way to do this?

I just honestly think you should use a shared_ptr. If an object needs to keep another alive, just use a shared_ptr. It saves you a lot of thinking with usually getting nowhere and spending time crafting a custom solution. Consider the cost of changing the architecture to not using a shared_ptr vs the time it takes to think up a custom solution and implement it.

I agree with you, Elysia, except for one reason: If the "owner" (now part-owner) of the Session holds a shared_ptr, then the Session can go out of scope but is destroyed only later (once the most recent call to PublishHandle::publish() has returned). I want to guarantee to the owner of the Session that the session is gone once it goes out of scope. So I need some kind of wait(), join(), block, etc.

Originally Posted by CodeMonkey

I want to guarantee to the owner of the Session that the session is gone once it goes out of scope. So I need some kind of wait(), join(), block, etc.

Don't. That will give you so many headaches.
You'd once again have to implement a system that broadcasts to all owners of the shared_ptr that this pointer must now be removed, and you must ensure mutual exclusion. It's just a pain and seriously, as far as the owner is concerned, the object is gone. Unless you run into memory issues, I'd ignore it and fix it later if it must be.

You're right, it's simpler to just use a shared_ptr, and so I think I will.

But for academic reasons alone, is there a simpler way to achieve what's below?

Code:

#include <memory>
#include <condition_variable>
#include <atomic>
#include <mutex>
#include <thread>

template <typename T>
struct default_deleter
{
    void operator() (T *obj) const
    {
        delete obj;
    }
};

template <typename T, unsigned n>
struct default_deleter<T[n]>
{
    void operator() (T(*arr)[n]) const
    {
        delete[] obj;
    }
};

class delete_notification
{
    std::mutex mutex;
    std::condition_variable cond;
    std::atomic<bool> isDeleted;
public:
    delete_notification()
        : isDeleted(false)
    {}

    void markDeleted()
    {
        if (!isDeleted.load()) {
            isDeleted = true;
            cond.notify_all();
        }
    }

    void waitUntilDeleted()
    {
        std::unique_lock<std::mutex> lock(mutex);
        while (!isDeleted.load())
            cond.wait(lock);
    }
};

template <typename T, typename Deleter = default_deleter<T> >
class notifying_deleter
{
    delete_notification & notification;
    Deleter del;
public:
    explicit notifying_deleter(delete_notification & notification,
        Deleter del = Deleter())
        : notification(notification), del(del)
    {}

    void operator () (T *obj) const
    {
        del(obj);
        notification.markDeleted();
    }
};

class delete_waiter
{
    delete_notification & notification;
public:
    explicit delete_waiter(delete_notification & notification)
        : notification(notification)
    {}

    ~delete_waiter()
    {
        notification.waitUntilDeleted();
    }
};

template <typename T>
class last_ptr
{
    delete_notification notification;
    delete_waiter waiter;
    const std::shared_ptr<T> sharedPtr;

public:
    template <typename Y>
    explicit last_ptr(Y *ptr)
        : waiter(notification),
        sharedPtr(ptr, notifying_deleter<T>(notification))
    {}

    template <typename Y, typename Deleter>
    last_ptr(Y *ptr, Deleter deleter)
        : waiter(notification),
        sharedPtr(ptr, notifying_deleter<T, Deleter>(notification, deleter))
    {}

    const std::shared_ptr<T> & shared() const
    {
        return sharedPtr;
    }
};

int main()
{
    last_ptr<int> last(new int(23));

    std::thread slowPoke([&last] {
        std::shared_ptr<int> ptr(last.shared());
        std::this_thread::sleep_for(std::chrono::seconds(5));
    });
    slowPoke.detach();

    std::shared_ptr<int> anotherRef(last.shared());

    return 0;
}

Oh boy - folks already trying to be clever with atomics...

There is a race between markDeleted() and waitUntilDeleted(). Git rid of the atomic and the use the mutex on both "sides" of the condition variable.

gg

I thought that might be a problem. Where is the race, though? My understanding is that when waitUntilDeleted() checks isDeleted.load(), a competing thread in markDeleted() must be on one side or the other of isDeleted = true;
Synchronization primitives are new to me, though.

Also, I think my array-valued deleter is incorrect. You don't call delete[] on a pointer to an array, you call it on a pointer that happens to point to the beginning of an array.

You just have to use your imagination to come up with an interleaving that breaks the logic. The key here is that calling notify* only wakes others that are currently blocked in a wait. It does not affect any future calls to wait.

Code:

Thread 1
    void markDeleted()
    {
A       if (!isDeleted.load()) 
        {
B           isDeleted = true;
C           cond.notify_all();
        }
    }

Thread 2    
    void waitUntilDeleted()
    {
        std::unique_lock<std::mutex> lock(mutex);
D       while (!isDeleted.load()) 
E           cond.wait(lock);
    }

Interleavings: D,ABC,E or A,D,BC,E or ...

Just need to have E occur after C.

gg

You just have to use your imagination to come up with an interleaving that breaks the logic. The key here is that calling notify* only wakes others that are currently blocked in a wait.

O_o

I don't feel right about either "deadlock" or "race condition".

You do get or miss the notification because of sequencing issues so "race condition".

You would also potentially lock multiple threads waiting on the same resource so "deadlock".

*shrug*

In any event, you can and should use `weak_ptr<???>' and `shared_ptr<???>' to do as described behind the scenes. The assertion that "shared_ptr isn't designed to do what I'm asking" is simply incorrect. You have a shared resource. The utility required is literally in the name. You have some clients of a shared resource which are not guaranteed ownership; the resource may or may not exist for such clients. You want a weak reference; once again, the utility required is literally in the name.

You can leave the wrapping classes if you want. I'd even suggest leaving the wrapping classes because the labels can inform about interface requirements, but you do want to use the `weak_ptr<???>' and `shared_ptr<???>' utilities. Your example approach even confirms what tools you should be using. Your code does nothing more and nothing less than attempt to forcibly keep a shared reference alive while the resource is still being accessed. The mechanism you are attempting to create is exactly what the conversion from a `weak_ptr<???>' to a `shared_ptr<???>' accomplishes.

Soma

No, phantomotap, I don't think what I'm trying to accomplish can be achieved without the wrapping classes. In addition to having shared ownership of the resource, a last_ptr also guarantees that it (the pointer) will not be destroyed until the shared resource is destroyed; further, it will block the thread trying to destroy it (the pointer) until the shared resource is destroyed.

Thanks for the examples, Codeplug.

And I'll be taking your advice, Elysia.

No, phantomotap, I don't think what I'm trying to accomplish can be achieved without the wrapping classes. In addition to having shared ownership of the resource, a last_ptr also guarantees that it (the pointer) will not be destroyed until the shared resource is destroyed; further, it will block the thread trying to destroy it (the pointer) until the shared resource is destroyed.

O_o

Except that blocking all other threads is not a real requirement to match the requirements you specified in other (#1 and #3) posts.

You certainly chose to implement the flawed thing in your post (#5) that way, but you do not have to and should not force other threads to block waiting on a shared resource to die solely because you require a single thread ("I want to guarantee to the owner of the Session that the session is gone once it goes out of scope.") to block until the shared resource dies.

In other words, your thinking about the solution is wrong because you've confused yourself into believing the solution has such a requirement as blocking on threads that do not own the resource.

[Edit]
*shrug*

I was trying to get you to make another pass at your code (#5) to understand where you went wrong as you say the code was an academic exercise, but you don't seem to want to take anything more from it so I also will not talk about.
[/Edit]

Soma