1

I am trying to write Manager class which is going to manage multiple instances of Test Class. I should be able to destroy an instance of Test Class by calling mng.drop(shared pointer to the instance to be dropped).

I am not supposed to use unique_ptr How do I implement using shared_ptr

#include <iostream>
#include <iomanip>
#include <memory>
#include <set>

#define DEBUG ON

#ifdef DEBUG
#define DEBUG_MSG(str) do {std::cout << std::setw(75) << std::left  << __FUNCTION__ \
    << std::setw(3) << std::left << ":" << std::setw(5) << std::left << __LINE__ \
    << std::setw(5) << std::left << ":"\
    << std::left  << str \
    << std::endl;} while( false )
#else
#define DEBUG_MSG(str) do { } while ( false )
#endif


class Test{
public:
    Test(int i) : i_(i){
        DEBUG_MSG("Constructor");
    }
    ~Test(){
        DEBUG_MSG("Destructor");
    }
    int getI() { return i_; }
    void setI(int i){ i_ = i; }
    void fn()
    {
        DEBUG_MSG("Do Something Here");
    }
private:
    int i_;
};

using sharedPtr = std::shared_ptr < Test >;


class Manager{
public:
    sharedPtr createTest(int i)
    {
        auto ptr = std::make_shared<Test>(i);
        list_.insert(ptr);
        return ptr;
    }

    void drop(sharedPtr ptr)
    {
        list_.erase(ptr);
    }

private:
    std::set<sharedPtr> list_;
};

int main()
{
    Manager mng;
    auto test = mng.createTest(50);
    DEBUG_MSG("test : " << test.use_count());
    test->fn();
    mng.drop(test);
    DEBUG_MSG("test : " << test.use_count());

    system("Pause");
    return 0;
}

As it can be seen : in my code when I call mng.drop(test) - still the reference count is 1, hence object is not destroyed.

Test::Test                                                                 :  22   :    Constructor
main                                                                       :  62   :    test : 2
Test::fn                                                                   :  31   :    Do Something Here
main                                                                       :  65   :    test : 1
Press any key to continue . . .

EDIT

My requirement: Manager Class should hold shared_ptr to all Test instances active; It should able to create and destroy Test instance
Improve this question
2
  • 1
    Why are you using shared_ptr if you want to manually control the lifetime of the objects? Commented Feb 19, 2015 at 9:13
  • @JorisTimmermans It is part of my bigger complex requirement. Was facing issue hence posted question making a very simplified scenario Commented Feb 19, 2015 at 9:16

3 Answers 3

Reset to default
3

The pointer returned by createTest and stored in test shares ownership with that managed by the manager; the object won't be destroyed until both are dropped.

createTest should return a weak_ptr if its clients are supposed to access the objects without sharing ownership. They can temporarily lock the pointer to prevent deletion when they need access; although you have to trust them not to permanently lock it, if it really is important for the manager to have this kind of "uniqueish" ownership for some reason.

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

I have come up with code like weakPtr createTest(int i) { auto ptr = std::make_shared<Test>(i); list_.insert(ptr); return weakPtr(ptr); } and int main() { Manager mng; auto test = mng.createTest(50); DEBUG_MSG("test : " << test.use_count()); test.lock()->fn(); DEBUG_MSG("test : " << test.use_count()); mng.drop(test.lock()); DEBUG_MSG("test : " << test.use_count()); system("Pause"); return 0; } What is the caveat of this type of code. I suppose after destruction weakPtr test is dangling.Should I reset weakPtr test?
@GauravK: It's not "dangling" as such. Weak pointers automatically become empty when the object is destroyed, so are still safe to use; unlike raw pointers, which are left pointing into the abyss with no way to tell that they're no longer valid. The caveat is that you should always test the result of lock() before dereferencing it. I'd also change drop to take a weak pointer, to save the client from having to lock it.
3

Your requirements don't make sense. If you need direct control over the objects' lifetime, do not use a smart pointer explicitly designed for indirect control (std::shared_ptr). Why do you have so technical requirements, anyway? Try to re-negotiate them.

If that is not an option, and you're willing to write code which will stick to the (illogical) requirements regardless of consequences, you can do this:

class Manager{
public:
    sharedPtr createTest(int i)
    {
        std::unique_ptr<Test> ptr(new Test(i));
        sharedPtr res(ptr.get(), [](Test*) {});
        list_.insert(std::move(ptr));
        return res;
    }

    void drop(sharedPtr ptr)
    {
        list_.erase(ptr.get()); // This requires C++14, or adapt list_ so that you can search by raw pointer in it
    }

private:
    std::set<std::unique_ptr<Test>> list_;
};

The code gives no-op deleters to the shared pointers. The object's lifetime is managed by the unique_ptr inside the Manager, but the clients can access the object through shared_ptr as required. Yes, this shared pointer can become dangling, but that's inherent in the requirements.

If the requirements on shared_ptr use are in fact negotiable, I see three ways to go:

  1. Have the Manager hold unique_ptr<Test>s and give out Test* (these are just fine as observers). Document that their lifetime is explicitly managed by the manager and they can therefore become dangling if held for too long.

  2. Follow @MikeSeymour's answer, have the Manager keep shared_ptrs and give out weak_ptrs. Document that the Manager can only guarantee destruction on drop() if clients behave and do not lock the weak_ptrs permanently.

  3. Give out smarts pointer similar to QPointer which get set to null when the object is destroyed.

I would prefer #1; to me, it feels as the simplest thing to do. Raw pointers are perfectly fine as observers, and if you have explicit lifetime management, you always run the risk of a dangling pointer somewhere. So just document it well and accept it.

Improve this answer

1 Comment

Given Test Class and Manager Class - and no compulsion on use of shared pointers; what would be best design for Manager Class. Can you give a sample code? If I change requirement to My requirement: Manager Class should list all Test instances active; It should able to create and destroy Test instance
0 
auto test = mng.createTest(50);
// here the count should be 2 and not 1
// one by the list's member and one by test
DEBUG_MSG("test : " << test.use_count());
test->fn();
mng.drop(test);
DEBUG_MSG("test : " << test.use_count());

in my code when I call mng.drop(test) - still the reference count is 1, hence object is not destroyed.

This is because you're still holding on to test, when you called mng.drop you give a copy of test to it, thereby bumping the reference count from n to n + 1. Now all it does it, decreases the reference count by 2: once for the local variable ptr and once for the one in the list, but the one you're holding still exists.

You've to release your reference to it too

test.reset();

This should make the reference count 0. You'll be able to see the destructor message getting printed.

Improve this answer

5 Comments

my drop(sharedPtr) should be able to destroy the object: My requirement: Manager Class should hold shared ptrs to all Test instances active; It should able to create and destroy Test instances
No; it will be able to reduce the reference count by 1 but that'll not lead to a 0 reference count and thus will not lead to a destruction since you're still holding on to it via test.
Exactly. How should I achieve this with my manager class, how do I design my code
@GauravK You don't. The requirements are inconsistent, they make no sense. You cannot require both shared lifetime control and explicit lifetime control at the same time.
@Angew: Agreed. Either have the manager have unique_ptr and let the clients use raw pointers IF you clearly know the manager (and the contained resource) outlives the client using it. Otherwise, go with Mike's way of giving out a weak_ptr to avoid meeting the said requirement.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.