Is there a reason why weak_ptr's cannot be compared with ==, or tested in a conditional? If I have two weak_ptr's, I thought it to be strange that I have to do: (w1.lock() == w2.lock()) And to test if a weak_ptr is the empty weak_ptr: if ( !(w1.lock()) ) which seems kind of awkward. Looking at the implementation, I see how the copy constructor is defined using lock. I understand what the comments are saying there. Perhaps there are some multithreaded issues that prevent equality testing I don't see. I would like the semantics that w1 and w2 compare to be equal whether or not the object they are pointing to are valid or not. I'm not sure if weak_ptrs become "empty" when the object they point to is destroyed (because there's no way to test for "emptyness" or "nullness" of a weak_ptr), but if they do I can see how the equality test can't work, but not the conditional test. Even so, why is there not an operator == provided that locks both weak_ptr? Is it acceptable (the documentation didn't make it clear to me), to use if ( w1.expired() ) to test if simply if w1 is "null" or "reset". What I am trying to do is have a "current" pointer that takes the value of something I want to be set (or note the fact that nothing is set), but I don't care if the object dies while that pointer is set -- in fact I don't want to keep it alive because I don't have an easy way to call reset on current were I to make it a shared_ptr. Jason
Jason Winnebeck wrote:
Is there a reason why weak_ptr's cannot be compared with ==, or tested in a conditional?
If I have two weak_ptr's, I thought it to be strange that I have to do:
(w1.lock() == w2.lock())
This comparison is well defined (even though the results you get may no longer be true if w1 or w2 is invalidated after the comparison) because the two lock() calls create two shared_ptr temporaries that "lock" the objects against deletion until operator== is evaluated. The general problem with weak_ptr is that its pointer can be invalidated at any time, and accessing its value would then be undefined behavior. A w1 == w2 shortcut is not provided since this kind of comparison isn't needed frequently and due to the subtleties involved.
And to test if a weak_ptr is the empty weak_ptr:
if ( !(w1.lock()) )
which seems kind of awkward.
Why do you want to test w1 without also doing something with it? [...]
I would like the semantics that w1 and w2 compare to be equal whether or not the object they are pointing to are valid or not. I'm not sure if weak_ptrs become "empty" when the object they point to is destroyed (because there's no way to test for "emptyness" or "nullness" of a weak_ptr), but if they do I can see how the equality test can't work, but not the conditional test.
The comparison you want is spelled !(w1 < w2) && !(w2 < w1), but why do you need it? [...]
Is it acceptable (the documentation didn't make it clear to me), to use
if ( w1.expired() )
to test if simply if w1 is "null" or "reset".
Yes, w1.expired() is an alias for w1.use_count() == 0, and use_count() is 0 after the default constructor or after reset(). It can also drop to zero after the last shared_ptr is destroyed.
What I am trying to do is have a "current" pointer that takes the value of something I want to be set (or note the fact that nothing is set), but I don't care if the object dies while that pointer is set -- in fact I don't want to keep it alive because I don't have an easy way to call reset on current were I to make it a shared_ptr.
But why do you need to compare weak_ptrs for that?
Peter Dimov wrote:
Why do you want to test w1 without also doing something with it?
But why do you need to compare weak_ptrs for that?
Perhaps this is a little too direct, but maybe my code can speak for itself to answer those questions and others from your reply. I mentioned earlier I was "converting" my network library from straight pointers to shared_ptr. The code here is from a network pong game example. The code made sense with pointers, but your comments suggest I may be misunderstanding the intent of the smart pointers. Previously I wrote this listener using PongClient* (which is a ConnectionListener). Now ConnectionListeners are managed by shared_ptr. I'm still undecided whether I should try to use shared_ptr for EVERYTHING in my game (the network code is but the game logic such as Player is not shared_ptr managed). The CV (ConditionVariable) has semantics that match pthread_cond with an associated mutex. The Lock* objects are RAII-style locks, I think they are most like scoped_lock from boost. The two "on" methods are event handlers, and getNewConnectionParams is an event called when a new connection has arrived but has not been negotiated. waitForPlayer is being called by the "main" thread in the server.
class OurListener : public ServerConnectionListener { public: typedef SmartPtr<OurListener> sptr; typedef WeakPtr<OurListener> wptr;
protected: OurListener(Player* RemotePlayer, Player* LocalPlayer) : remotePlayer(RemotePlayer), localPlayer(LocalPlayer), accept(true) { }
public: /** * This listener takes the params it needs to pass them onto the PongClient * to set up the game. */ static sptr create( Player* RemotePlayer, Player* LocalPlayer ) { sptr ret( new OurListener( RemotePlayer, LocalPlayer ) ); ret->setThisPointer( ret ); return ret; }
virtual ~OurListener() {}
void onListenFailure(const Error& error, const Address& from, const ConnectionListener::sptr& listener) { LockCV lock(sync);
if (listener == connecting.lock()) { //Only display an error for our real player. We don't want to see the //ConnectionRefused errors. LockObject lock( gout ); gout << "Connection error: " << error << endl; gout << " Error received from " << from << endl; connecting.reset(); }
//If waitForPlayer is waiting for the connection, wake it up. sync.broadcast(); }
void onListenSuccess( const ConnectionListener::sptr& listener ) { LockCV lock(sync);
player = connecting; connecting.reset(); accept = false;
//If waitForPlayer is waiting for the connection, wake it up. sync.broadcast(); }
void getNewConnectionParams(ConnectionParams& params) { LockCV lock(sync);
params.setUnrel(false); if (accept && !(connecting.lock()) ) { //If no one is connecting and we are accepting connections PongClient::sptr temp = PongClient::create( remotePlayer, localPlayer ); connecting = temp; params.setListener( temp ); } else { params.setListener( RefuseClient::create() ); } }
//waitForPlayer returns the connected player, or NULL if the connection //process was aborted by the user. PongClient::sptr waitForPlayer() { LockCV lock(sync);
while (!(player.lock()) && !kbhit()) { //We wait for 250ms to recheck kbhit for pressed keys. sync.timedWait(250); } if (!(player.lock())) { //We were woken up by a keypress, so refuse any further connections. accept = false;
//We don't need to wait around if anyone is in the middle of connecting, //because shutting down GNE will close any open connections, and we will //be closing down if we aborted this connection.
//Return that no client connected. return PongClient::sptr(); }
return player.lock(); }
private: Player* remotePlayer; Player* localPlayer;
//This variable will be non-null when there is a player, so we refuse any //other incoming connections. Weak pointer used because we don't want to //keep player alive after connect PongClient::wptr player;
//player will be stored here while he is connecting, then moved to player //when the connection was successful. Weak pointer used because this //variable is only temporary and we don't want to keep anything alive. PongClient::wptr connecting;
//If this is false, then the user canceled the connection process, we we //shouldn't even accept the first player. bool accept;
//We use a CV because getNewConnectionParams might be called from //different threads, and we want to make sure only one client connects at a //time. This protects the state of player. //The CV is notified when a new player arrives. ConditionVariable sync; };
Jason
From: "Jason Winnebeck"
//waitForPlayer returns the connected player, or NULL if the connection //process was aborted by the user. PongClient::sptr waitForPlayer() { LockCV lock(sync);
while (!(player.lock()) && !kbhit()) { //We wait for 250ms to recheck kbhit for pressed keys. sync.timedWait(250); } if (!(player.lock())) { //We were woken up by a keypress, so refuse any further connections. accept = false;
//We don't need to wait around if anyone is in the middle of connecting, //because shutting down GNE will close any open connections, and we will //be closing down if we aborted this connection.
//Return that no client connected. return PongClient::sptr(); }
return player.lock(); }
Unfortunately I don't have the time to properly review your code. I'll just rewrite the above in a more "idiomatic" way (I hope I'm not missing something.) PongClient::sptr waitForPlayer() { LockCV lock(sync); for(;;) { if( PongClient::sptr p = player.lock() ) { return p; } else if( kbhit() ) { accept = false; return PongClient::sptr(); } else { sync.timedWait(250); } } } Even more "idiomatic" would be to use shared_ptr<...> instead of the ::sptr typedef. Hiding the type of the smart pointer behind a typedef usually doesn't help much and can lead to subtle errors since the semantics of the various smart pointers differ in some corner cases. HTH
participants (2)
-
Jason Winnebeck
-
Peter Dimov