Circular references between two classes

Question

I know this must be a n00b question, but I have to implement a mockup client-server sequential interaction application, and because the number of client-server calls varies, I cannot just iterate the steps in an external function, always fetching data from the client and then forwarding it to the server and vice-versa, so I need to make my Server and Client classes be aware of each other so that they can call their public methods between themselves. One approach would be to design both as Singletons, but I was hoping to do it in a simpler way, more precisely using a circular reference: the client stores a reference to the server and the server stores a reference to the client. I am aware that this might not be a good approach, and it could cause the call stack to explode when it becomes too deep, so any improvements to my design are welcomed.

In order to achieve the described implementation, I thought that I could use std::shared_ptr, because std::unique_ptr won't work if I also want to prevent the two variables from main from getting clobbered when I call the two setters (right?). So, this is what I have (simplified code):

#include <iostream>
#include <memory>

class Server;

class Client
{
public:
    void SetServer (const Server &server);
private:
    std::shared_ptr<const Server> server;
};

void Client::SetServer (const Server &server)
{
    this->server = std::shared_ptr<const Server>(&server);
}

class Server
{
public:
    void SetClient (const Client &client);
private:
    std::shared_ptr<const Client> client;
};

void Server::SetClient (const Client &client)
{
    this->client = std::shared_ptr<const Client>(&client);
}

int main ()
{
    Server server;
    Client client;

    server.SetClient(client);
    client.SetServer(server);

    //Here I ask the client to start interacting with the server.
    //The process will terminate once the client
    //exhausts all the data it needs to send to the server for processing

    return 0;
}

Unfortunately, my code seems to try to call the Client and Server (implicit) destructors multiple times, or some similar nasty thing, and I'm certain that this is caused by my poor understanding of how std::shared_ptr is supposed to work. Please advise.

Tetrabasic

+1 for a self-contained, complete sample program. sscce.org. – Quickfreeze

You're passing the Server and Client arguments to SetXxxx as references. That's not how one would do it usually. I'd think it idiomatic to pass those as pointers. Ideally, as intelligent pointers, not naked pointers. – Teetotalism

@KubaOber I will try to work it out. Thanks for pointing out the issue. – Tetrabasic

Answer 1

You allocate your Server and Client instances on the stack, and they will be deleted when main() exits. You don't want std::shared_ptr deleting them as well. Thus, there are two solutions:

1. Use unmanaged pointers within Client and Server, and manage their lifetimes externally.

2. Use managed pointers everywhere, including main(). Do note that shared_ptr implies ownership. In the current design, a Server owns the client, but also a Client owns the server. That's a circular reference: by default, they'll never be freed, unless you reset one of those pointers while you still can.

   As an example, you can decide that the clients keep the server alive, thus last vanishing client will bring down the server if there are no other shared_ptrs pointing at it. The server would have a weak_ptr to the client(s), but the clients would have a shared_ptr to the server.

class Client;
class Server;

class Client
{
public:
    void SetServer (const std::shared_ptr<const Server> &server);
private:
    std::shared_ptr<const Server> server;
};

void Client::SetServer (const std::shared_ptr<const Server> &server)
{
    this->server = server;
}

class Server
{
public:
    void SetClient (const std::weak_ptr<const Client> &client);
private:
    std::weak_ptr<const Client> client;
};

void Server::SetClient (const std::weak_ptr<const Client> &client)
{
    this->client = client;
}


int main()
{
  std::shared_ptr<Server> server(new Server);
  std::shared_ptr<Client> client(new Client);

  server->SetClient(client);
  client->SetServer(server);

  // do stuff

  return 0;
}

Pietra

+1. The important thing is to keep in mind that you surrender the responsibility for deletion of the object pointed by what you used to construct the std::shared_ptr. – Creaky

You can't construct a weak_ptr from a raw pointer. You can only construct a weak_ptr from a shared_ptr or another weak_ptr. – Bridgeman

@KubaOber Yeah, that's really nice and it seems to work. Thanks! Unfortunately, I feel stupid to just plug&play code that I do not understand properly. Could you please elaborate on the details, like, for example what happens behind the curtains and why use std::weak_ptr? What dictates the order in which the pointers get released and the destructors get called? If we are to assume that the server is and always will be just one, my common sense tells me that I should be using an std::unique_ptr instead. – Tetrabasic

Also, another stupid question: why not use std::weak_ptr for the internal server reference in the Client class as well? I read through this article msdn.microsoft.com/en-us/library/bb982126.aspx but the answer to this question still eludes me. – Tetrabasic

Nevermind. I found a good explanation here https://mcmap.net/q/183296/-shared_ptr-and-weak_ptr-differences :) – Tetrabasic

You could, but you have to decide who owns what. If you decided that server is owned externally (say by int main()), then of course weak pointers in the clients would do the trick. – Teetotalism

You said that the server should have weak+ptr to the clients, but when I try something similar, the weak_ptr has no pointer operator so I can not access the member methods of the clients from the server. – Delacourt

@Delacourt The meaning of a weak pointer is that you can't use it for anything other than converting it to a shared pointer. Thus convert it to a shared pointer using the lock() method, and if it's non-null you can use it. A non-null shared pointer guarantees that the object will exist for as long as the pointer itself exists. A weak pointer guarantees nothing and thus can't be used directly as a pointer. – Teetotalism

@Kuba Ober - that doesn't help me. I need to be able to access member functions. But I figured out what was happening. I did some more research and discovered that I should in fact be using standard pointers to pass into functions and maintain references to parent/child classes. Once I did that, the destructors started getting called. – Delacourt

Answer 2

Your variables have automatic lifetime, and will be destroyed when they go out of scope.

Therefore, using any of the lifetime-managing smart pointers is wrong, because it will call delete a second time.

Tabbie