Mutual dependency of two objects

Viewed 297

Quite frequently, I stuble over a situation like this: two objects need to know each other, and we have a mutual aggregation-style dependency (imagine, for example, one object handles a websocket connection, and the other handles a dbus connection, and we need to forward messages in both directions). A UML diagram would look like that:

UML class diagram of mutual aggregation style dependency

A simple way to create this dependency in C++ would be to just pass pointers to each other:

int main() {
  TypeA a;
  TypeB b;

  a.SetB(&b);
  b.SetA(&a);

  // ...
}

I see a potential memory problem here. When main() returns, first b is destroyed, then a. Between those two steps, a might still be running in another thread and access the pointer to b, which is invalid at this time, causing a seg-fault.

My current solution to that problem is using C++11 smart pointers. Both TypeA and TypeB store weak_ptr to the other, and must always check if the pointer is valid before accessing it:

int main() {
  auto a = std::make_shared<TypeA>();
  auto b = std::make_shared<TypeB>();

  a->SetB(b);    // this method converts the shared_ptr to a weak_ptr
  b->SetA(a);    // this method converts the shared_ptr to a weak_ptr

  // ...
}

I am unsure if this really is a proper solution. Also, I'm not so happy that the objects always must be on the heap, and I cannot just place them on the stack anymore.

Can anyone imagine another solution? How to solve this in C++98 or in C?

4 Answers

You could define a third class C such that

  • C knows A and B
  • A knows C
  • B knows C

When either A or B do their job, they inform C, which would then forward the job to the other class if it is possible. With this scheme you can extend it to more classes.

The way I understand your problem, both classes are running their own, internal thread and would crash if the enclosed pointer becomes invalid.

While you have TypeA::setB(typeB*) and TypeB::setA(typeA*) methods in the objects, the trick would be to also have synchronized TypeA::deregisterB(typeB*) and TypeB::deregisterA(typeA*) methods which you would call in the objects destructors. That way you get rid of your memroy issues.

class TypeB;

class TypeA {
   public:
       TypeA() = default;

       ~TypeA() {
           // stopThread
           if (_b) {
               _b->deregisterA(this);
           }
       }

       void idle() {
           ...
       }

       void setB(TypeB* b) {
           _b = b;
       }

       /**
        * Disconnects _b from this.
        * TypeB* : Object to deregister. A parameter is only required
        *          if TypeA has multiple pointers to TypeB.
        */
       void derigisterB(TypeB* b) {
           // ... wait for a save moment to delete b
           _b = nullptr;
       }

   private:
       TypeB* _b = nullptr;
};

class TypeB {
    // ... same as TypeA
}

Regarding your second question. You need to think about the ownership of the pointers. All you need to be sure about is that their lifetime is controlled and they get deleted at the right point of time. If you have something that takes care if this, you can just ditch die weak_ptr and pass a raw pointer instead:

int main() {
  TypeA a;
  TypeB b;

  a.SetB(&b);    // pass address of b
  b.SetA(&a);    // pass address of a

  // ...

  // b will be deleted first. Its destructor calls a->deregisterB(this)
  // method which sets a's pointer to b to nullptr.
  // a will get deleted last. As it already knows there is no
  // more b, it does not need to call deregisterA(this) on b.
}

First, you should make sure that dependency really is between the objects rather than the method calls. Perhaps objects don't necessarily have to hold pointers to each other, you could pass the relevant object to the called method.

If you do have a mutual object dependency (as is sometimes the case) figure out if one of the two objects owns the other. This can happen often when you try to model a problem using classes: for instance, a Window class owns a RenderingContext, because the rendering context cannot exist if the window is closed/destroyed. In this case, the owned class should really hold just a regular pointer to the owner.

Sometimes, two objects will need to refer to each other. In this case, using smart pointers might be what you want. In such a case, though, you don't need both pointers to be std::weak_ptr, just one, because one weak pointer is sufficient in order to break the dependency cycle.

Regarding your multithreading concern, you might want to look into the delete this; idiom. https://isocpp.org/wiki/faq/freestore-mgmt#delete-this

When both objects run in their own thread, you could use channels (or pipes, or queues, or however you want to call them) for the communication between those two objects.

You create a channel for each object and pass references to them as sending and receiving ends respectively to the objects. The objects than can listen on their receiving end for messages as actors and act upon new messages when they receive and take them. This breaks up the cyclic dependency, as each object now holds references to the channels over which they communicate with each other.

Channel:

// The interface for the sending end of a Channel
template<typename T>
class SendingChannel {
  public:
    virtual void send(T) = 0;

    virtual ~SendingChannel() = default;
};


// The interface for the receiving end of a Channel
template<typename T>
class ReceivingChannel {
  public:
    virtual T receive() = 0;

    virtual ~ReceivingChannel() = default;
};


// The implementation for a whole Channel
template<typename T>
class Channel: public SendingChannel<T>, public ReceivingChannel<T> {
  private:
    std::queue<T> msgs{};
    std::mutex channel_mtx{};
    std::condition_variable receiving_finishable{};

  public:
    bool is_empty() const { return msgs.empty(); }
    bool is_not_empty() const { return !is_empty(); }

    void send(T msg) override {
        std::lock_guard channel_lck{channel_mtx};

        msgs.push(std::move(msg));
        receiving_finishable.notify_one();
    }

    T receive() override {
        std::unique_lock channel_lck{channel_mtx};
        receiving_finishable.wait(
            channel_lck, 
            [this](){ return is_not_empty(); }
        );

        T msg{std::move(msgs.front())};
        msgs.pop();

        return msg;
    }
};

The message used for communication between the objects might consist of en enum to denote the type and maybe a variant for the ability to transport values of different types. But different values and action types could also be conveyd by polymorphic message types or the function of the functial standard library.

Execution loop of an actor:

while (true) {
    auto message = inbox.receive();

    switch (message.type) {
        case MsgType::PrintHello:
            print_hello();
            break;
        case MsgType::PrintMessage:
            print_message(get<std::string>(message.argument));
            break;
        case MsgType::GetValue:
            send_value();
            break;
        case MsgType::Value:
            print_value(get<int>(message.argument));
            break;
    }
}

main:

int main() {
    Channel<Message> to_b;
    Channel<Message> to_a;

    Object a("A", to_a, to_b);
    Object b("B", to_b, to_a);

    thread thread_a{a};
    thread thread_b{b};

    to_a.send(Message{MsgType::PrintMessage, "Hello, World!"});
    to_b.send(Message{MsgType::PrintHello});
    
    thread_a.join();
    thread_b.join();
}

As you can see in main, there is no need for any pointers, nothing needs to be declared on the heap and there are no cyclic references. Channels are a decent solution to isolate threads and the objects running on them. The actors can act and communicate over the channels thread safe.

My full example can be viewed on my Github repo.

Related