Advice on weak_ptr usage

Started by
8 comments, last by Alberth 8 years, 7 months ago

Hello all.

I'm about to add the network class to the objects that need to send information to the server.

By adding a weak_ptr of the network class thats is a shared_ptr into the base object.

Is the weak pointerer ok for this what I'm lead to believe, is that if the network layer is deleted which it can be at any time when disconnect happens.

that doing this

if( auto tmp = weak1.lock() )

//good to use

else

//has been deleted

will stop any use of the class after its been deleted, but no object will be able to keep the shared_ptr alive which would be bad if a dissconnect occurs.

This is my first use for a weak_ptr.

So will the weak_ptr protect me from a deleted network class. Has to be better then a raw pointer.

I was thinking of passing the network class to functions but there is 17 functions and some functions are deep down in sub systems.

never lead to a clean route.

or any other way to pass network class to things that need them.

Advertisement

Wouldn't a single point of entry to 'the network' that always exists be more useful?

Such a object could then have a connection (or not) to the server.

All other objects that need to send information ask the network class to send things, which can respond with 'no connection', or 'sent', or whatever is appropriate in your case.

You can reference directly to the connection object, but that scatters your connection information all over your objects. Say the server disconnects (and some of your objects find out, and others have not yet), and the server re-connects. You have to update the all objects with the new connection.

If you make a network object, you can just update the connection in the network object, and you're done.

In general, I think weak pointers are not for delete detection (you should handle that by yourself, imho). Weak pointers are often used for detecting that nobody needs the information any more. For example, in caches you don't want to cache data that nobody needs.

All other objects that need to send information ask the network class to send things

Thats what the weak_ptr is for weak_ptr<InetClient> Client2GameServer;

then use

Client2GameServer->Send(.....); this will post to a send queue.

some thing like SetDestination() will want to send to the server the point the user clicked and some other stuff.

but SetDestination is a Human control for the object and is controled in a BuildButtonMenuManager. its a bunch of buttons on a RTS interface for creating objects but its complex and it looks like the network layer is kept in one place by alowing a object to have access to the network layer in its base clase. And that way where ever the object is in what subsystem it can have the network layer.

But thats where the weak_ptr was going to prevent sends on a NULL pointer

Weak pointers are often used for detecting that nobody needs the information any more

thats the same thing just using it in a different way.

I don't want a shared_ptr because all objects would need to be removed for the pointer to go out of scope and delete its self.

Raw point to raw.

Just done some more reading

Weak pointers just "observe" the managed object; they don't "keep it alive" or affect its lifetime. Unlike

shared_ptr

s, when the last weak_ptr goes out of scope or disappears, the pointed-to object can still exist because

the

weak_ptrs do not affect the lifetime of the object - they have no ownership rights. But the weak_ptr can be

used to determine whether the object exists, and to provide a

shared_ptr that can be used to refer to

Why are you deleting your network class at all?

Conceptually, the "network" still exists, it just may have lost the connection. By storing the connection state instead of deleting the entire object you can easily reconnect without then having to go through everyone that might have a pointer to the network class and telling them to point at your new "network" that you allocated for the new connection.

Yes, weak_ptr is generally an observer pointer of a shared_ptr that may go away while others want to look at it - but I don't think it's the right model for what you're doing here.

It can be argued that use of shared_ptr and weak_ptr is a failure of the design, in that you have chosen not to have tight lifetime control. You should be able to know when things exist and when they don't, and be able to clearly state who owns what. Of course, there are valid designs that use them, but they provide weaker guarantees and tend to promote more "spaghetti-like" architecture than designs that don't rely on them.
I'll concur. weak_ptr does work for what you're asking for, but you're asking for the wrong thing.

Consider this superior alternative API:

enum class ClientId : uint32 { none };

class Network {
public:
  // determine if a given ClienId refers to a valid client
  bool IsClientConnected(ClientId client) const;

  // disconnect a client and flush all outgoing messages to them
  void DisconnectClient(ClientId client);

  // queue a message to be sent to a given client
  void SendToClient(ClientId, NetworkMessage msg);

  // process all incoming connections and received messages (also flush outgoing messages)
  void Update(std::function<void(CleintId)> connectCallback,
              std::function<void(ClientId, NetworkMessage)> receiveCallback,
              std::function<void(ClientId)> droppedCallback);
};
That API is much simpler, requires far fewer public classes, avoids excessive bad object hierarchies, and reduces the complexity of lifetime management of connections. So long as you aren't reusing ClientIds willy-nilly (pretty darn easy, since it's a 32-bit number with ~4 billion posible values), there's no problem just having your game code hold on to a nice simple little ClientId value where appropriate.

I don't make this recommendation on a whim, either; I literally replaced an MMO's network engine written with smart pointers and the like into a (slightly more production-quality) version of the above back in March.

but SetDestination is a Human control for the object and is controled in a BuildButtonMenuManager. its a bunch of buttons on a RTS interface for creating objects but its complex and it looks like the network layer is kept in one place by alowing a object to have access to the network layer in its base clase. And that way where ever the object is in what subsystem it can have the network layer.


That is not what base classes are for. Just make the network system be a reference the object holds on to, if you must, or go "crazy" and use a global/singleton. They're not evil, I promise. There comes a point when trying to avoid globals and singletons causes more complexity and bloated code/objects than is worth avoiding the minor inconveniences of singletons that you're almost certainly not dealing with anyhow (are you writing comprehensive test suites with mock systems for your game code? thought not).

Sean Middleditch – Game Systems Engineer – Join my team!

I tend to agree that this is something of a design smell, but your functional use of weak_ptr is more or less correct -- weak pointers can be used when you need to share an object among observers who do not need to keep the observed object alive; that is, they can deal with it if the object dies (for example, they can then reacquire a new instance of what was observed). Again, design-wise this seems less than great, but functionally you're using weak_ptr in one of the ways it can be used.

A more typical use of weak_ptr is for breaking referential cycles (two objects have a shared pointer to one another that prevents either from ever being destructed) -- by using weak_ptr, destruction is not prevented and the cycle is broken.

A better pattern for what you're doing might be to pass in a network instance to a per-object network handling function inside each object (or the inverse, pass objects per-frame into a network object).

throw table_exception("(? ???)? ? ???");

Hey thanks for all the tips.

Have a new idea now, What about if I define a function declaration in a header SendOnly.h will have a function declaration only and the body can be defined in main.cpp where the network class is

SendOnly.h

//----------------------------------------------------------------------------------------------

//define the body of this function in main.cpp

//alows us to post messages to the network layer from anywhere in the

//code base

//-----------------------------------------------------------------------------------------------

void PostNetworkMessage(std::string &serializeddata);

//------------------------------------------------------------------------------------------------

//then in main define the function body

//allows us to send messages to the server

//------------------------------------------------------------------------------------------------

void PostNetworkMessage(std::string &serializeddata)

{

if(InetClient2Server.get() == NULL)

return;//network layer down not a error could be a off line game errors are posted by network layer to main app

//ok network layer active send message to server we can encrypt per send

InetClient2Server->Send(serializeddata,

DELIVER_USE_PUBLICKEY,//bool usingPrivateKey,//we need to know if we are server private = true client public false

DELIVER_USE_COMPRESSION,//true is we want to compress the data

DELIVER_USE_ENCRYPTION,//true if we want to encrypt the data

DELIVER_USE_DATASIGNATURE,//true if we want to create a digest DataSignature

DELIVER_USE_RSASIGNING);//true to use rsa public or private keys we sign the DataSignature);

}

I think the issue is that you are equating pointers with connections.

You can have a file object, then close a file, the file object does not suddenly become invalid. It remains a valid object with no open file. You can use that valid object to open a new connection to another file.

Similarly with a networking object, you can close the connection this does not delete the object. It remains a valid object with no open connection. You can use that valid object to open a new connection to another network endpiont.


Just like writing to a file can sometimes fail with an error such as being out of disk space, writing to a network can also sometimes fail. In both cases this is handled through an error code. If a network connection goes bad you might want to attempt to reconnect, or cancel the connection and give the reasons to the user. It doesn't mean the code needs to destroy the networking objects. You can, but you don't really need to.

No for some reason I thought it a good Idea to delete the client when a disconnect happens hehehe.

I tryed the extern function setup and its working well and I don't need to include the network header in any other library headers now which is a good thing

(boost::asio and windows.h and dx10.h has windows.h has winsock.h and a lot of omg wtfblink.png blink.png )

Just have to include the SendOnly.h header. which has benefits as you can see them flags in the send declaration now can be changed in the one function instead of being all over the place.

The Game design was started 9 years ago, the only thing I new for networking for this project is it was going to be networked. I think it plugged in so far well this was the only tricky bit the menu system worked great like it was made for it. I did have to change some but most turned out well.

At the moment it now creates new objects and sends the remote object for all clients, this bit was to send destination to get the remote object and the clients object moving.

So with that all said and done. Should I use the current extern function(You dont need the extern key word any more new to me not needed in c++ hehe been using the key word in the past in c++ started with c but still think c most times working on it)

The 'extern' keyword has not changed much between c and c++. Already in c, you only needed 'extern' for data declarations where you don't want memory reserved for it. You never needed to use 'extern' for function declarations, although some people did for clarity.

'extern' typically used in header files to declare (but not allocate) global data.
One .c/.cpp file then has the same declaration, but without 'extern' to allocate the data.

This topic is closed to new replies.

Advertisement