Merge pull request #75 from eile/master

Fix #29: Change managers may access dangling objects

.travis.yml

@@ -7,13 +7,13 @@ script:
  - mkdir Debug
  - cd Debug
  - cmake .. -DCI_BUILD_COMMIT=$TRAVIS_COMMIT -DCMAKE_BUILD_TYPE=Debug -DTRAVIS=1
- - env TRAVIS=1 make -j8 tests ARGS=-V
+ - env TRAVIS=1 make -j8 cis ARGS=-V
  - mkdir ../Release
  - git status
  - git --no-pager diff
  - cd ../Release
  - cmake .. -DCI_BUILD_COMMIT=$TRAVIS_COMMIT -DCMAKE_BUILD_TYPE=Release -DTRAVIS=1
- - env TRAVIS=1 make -j8 tests ARGS=-V
+ - env TRAVIS=1 make -j8 cis ARGS=-V
  - git status
  - git --no-pager diff
 before_install:

co/fullMasterCM.cpp

@@ -1,5 +1,5 @@
 
-/* Copyright (c) 2007-2013, Stefan Eilemann <[email protected]>
+/* Copyright (c) 2007-2014, Stefan Eilemann <[email protected]>
  *               2011-2012, Daniel Nachbaur <[email protected]>
  *
  * This file is part of Collage <https://github.com/Eyescale/Collage>
@@ -164,7 +164,7 @@ void FullMasterCM::_obsolete()
     _checkConsistency();
 }
 
-void FullMasterCM::_initSlave( const MasterCMCommand& command,
+bool FullMasterCM::_initSlave( const MasterCMCommand& command,
                                const uint128_t& /*replyVersion*/,
                                bool replyUseCache )
 {
@@ -254,6 +254,7 @@ void FullMasterCM::_initSlave( const MasterCMCommand& command,
         LBINFO << "Cached " << _hit << "/" << _hit + _miss
                << " instance data transmissions" << std::endl;
 #endif
+    return true;
 }
 
 void FullMasterCM::_checkConsistency() const
@@ -379,7 +380,7 @@ void FullMasterCM::push( const uint128_t& groupID, const uint128_t& typeID,
     instanceData->os.push( nodes, _object->getID(), groupID, typeID );
 }
 
-void FullMasterCM::sendSync( const MasterCMCommand& command )
+bool FullMasterCM::sendSync( const MasterCMCommand& command )
 {
     //const uint128_t& version = command.getRequestedVersion();
     const uint128_t& maxCachedVersion = command.getMaxCachedVersion();
@@ -399,6 +400,7 @@ void FullMasterCM::sendSync( const MasterCMCommand& command )
     node->send( CMD_NODE_SYNC_OBJECT_REPLY, useCache /*preferMulticast*/ )
         << node->getNodeID() << command.getObjectID() << command.getRequestID()
         << true << command.useCache() << useCache;
+    return true;
 }
 
 }

co/fullMasterCM.h

@@ -1,5 +1,5 @@
 
-/* Copyright (c) 2007-2013, Stefan Eilemann <[email protected]>
+/* Copyright (c) 2007-2014, Stefan Eilemann <[email protected]>
  *               2011-2012, Daniel Nachbaur <[email protected]>
  *
  * This file is part of Collage <https://github.com/Eyescale/Collage>
@@ -45,7 +45,7 @@ namespace co
         uint128_t commit( const uint32_t incarnation ) override;
         void push( const uint128_t& groupID, const uint128_t& typeID,
                    const Nodes& nodes ) override;
-        void sendSync( const MasterCMCommand& command ) override;
+        bool sendSync( const MasterCMCommand& command ) override;
 
         /** @name Versioning */
         //@{
@@ -66,9 +66,8 @@ namespace co
             uint32_t commitCount;
         };
 
-        void _initSlave( const MasterCMCommand& command,
-                         const uint128_t& replyVersion,
-                         bool replyUseCache ) override;
+        bool _initSlave( const MasterCMCommand&, const uint128_t&,
+                         bool ) override;
 
         InstanceData* _newInstanceData();
         void _addInstanceData( InstanceData* data );

co/localNode.cpp

@@ -1,5 +1,5 @@
 
-/* Copyright (c) 2005-2013, Stefan Eilemann <[email protected]>
+/* Copyright (c) 2005-2014, Stefan Eilemann <[email protected]>
  *                    2010, Cedric Stalder <[email protected]>
  *                    2012, Daniel Nachbaur <[email protected]>
  *
@@ -659,61 +659,56 @@ void LocalNode::disableSendOnRegister()
 
 bool LocalNode::registerObject( Object* object )
 {
-    return _impl->objectStore->registerObject( object );
+    return _impl->objectStore->register_( object );
 }
 
 void LocalNode::deregisterObject( Object* object )
 {
-    _impl->objectStore->deregisterObject( object );
+    _impl->objectStore->deregister( object );
 }
 
 f_bool_t LocalNode::mapObject( Object* object, const UUID& id, NodePtr master,
                               const uint128_t& version )
 {
-    const uint32_t request = _impl->objectStore->mapObjectNB( object, id,
-                                                              version, master );
-    const FuturebImpl::Func& func = boost::bind( &ObjectStore::mapObjectSync,
+    const uint32_t request = _impl->objectStore->mapNB( object, id, version,
+                                                        master );
+    const FuturebImpl::Func& func = boost::bind( &ObjectStore::mapSync,
                                                  _impl->objectStore, request );
     return f_bool_t( new FuturebImpl( func ));
 }
 
 uint32_t LocalNode::mapObjectNB( Object* object, const UUID& id,
                                  const uint128_t& version )
 {
-    return _impl->objectStore->mapObjectNB( object, id, version, 0 );
+    return _impl->objectStore->mapNB( object, id, version, 0 );
 }
 
 uint32_t LocalNode::mapObjectNB( Object* object, const UUID& id,
                                  const uint128_t& version, NodePtr master )
 {
-    return _impl->objectStore->mapObjectNB( object, id, version, master );
+    return _impl->objectStore->mapNB( object, id, version, master );
 }
 
 
 bool LocalNode::mapObjectSync( const uint32_t requestID )
 {
-    return _impl->objectStore->mapObjectSync( requestID );
+    return _impl->objectStore->mapSync( requestID );
 }
 
 f_bool_t LocalNode::syncObject( Object* object, NodePtr master, const UUID& id,
                                const uint32_t instanceID )
 {
-    const uint32_t request = _impl->objectStore->syncObjectNB( object, master,
-                                                               id, instanceID );
-    const FuturebImpl::Func& func = boost::bind( &ObjectStore::syncObjectSync,
-                                                 _impl->objectStore, request,
-                                                 object );
-    return f_bool_t( new FuturebImpl( func ));
+    return _impl->objectStore->sync( object, master, id, instanceID );
 }
 
 void LocalNode::unmapObject( Object* object )
 {
-    _impl->objectStore->unmapObject( object );
+    _impl->objectStore->unmap( object );
 }
 
 void LocalNode::swapObject( Object* oldObject, Object* newObject )
 {
-    _impl->objectStore->swapObject( oldObject, newObject );
+    _impl->objectStore->swap( oldObject, newObject );
 }
 
 void LocalNode::objectPush( const uint128_t& groupID,

co/localNode.h

@@ -1,5 +1,5 @@
 
-/* Copyright (c) 2005-2013, Stefan Eilemann <[email protected]>
+/* Copyright (c) 2005-2014, Stefan Eilemann <[email protected]>
  *                    2010, Cedric Stalder <[email protected]>
  *                    2012, Daniel Nachbaur <[email protected]>
  *
@@ -295,7 +295,7 @@ class LocalNode : public lunchbox::RequestHandler, public Node,
      */
     CO_API f_bool_t syncObject( Object* object, NodePtr master,
                                 const UUID& id,
-                                const uint32_t instanceID=CO_INSTANCE_ALL );
+                                const uint32_t instanceID = CO_INSTANCE_ALL );
     /**
      * Unmap a mapped object.
      *

co/nullCM.h

@@ -1,5 +1,5 @@
 
-/* Copyright (c) 2007-2013, Stefan Eilemann <[email protected]>
+/* Copyright (c) 2007-2014, Stefan Eilemann <[email protected]>
  *               2011-2012, Daniel Nachbaur <[email protected]>
  *
  * This file is part of Collage <https://github.com/Eyescale/Collage>
@@ -41,7 +41,8 @@ namespace co
 
         void push( const uint128_t&, const uint128_t&,
                    const Nodes& ) override { LBDONTCALL; }
-        void sendSync( const MasterCMCommand& ) override { LBDONTCALL; }
+        bool sendSync( const MasterCMCommand& ) override
+            { LBDONTCALL; return false; }
 
         uint128_t getHeadVersion() const override
             { return VERSION_NONE; }
@@ -50,7 +51,8 @@ namespace co
         uint32_t getMasterInstanceID() const override
             { LBDONTCALL; return CO_INSTANCE_INVALID; }
 
-        void addSlave( const MasterCMCommand& ) override { LBDONTCALL; }
+        bool addSlave( const MasterCMCommand& ) override
+            { LBDONTCALL; return false; }
         void removeSlaves( NodePtr ) override { LBDONTCALL; }
 
     private:

co/object.cpp

@@ -1,5 +1,5 @@
 
-/* Copyright (c) 2005-2013, Stefan Eilemann <[email protected]>
+/* Copyright (c) 2005-2014, Stefan Eilemann <[email protected]>
  *               2011-2012, Daniel Nachbaur <[email protected]>
  *
  * This file is part of Collage <https://github.com/Eyescale/Collage>
@@ -85,8 +85,8 @@ Object::~Object()
 
     if( impl_->localNode )
         impl_->localNode->releaseObject( this );
-    impl_->localNode = 0;
 
+    impl_->localNode = 0;
     impl_->cm = 0;
     delete impl_;
 }
@@ -181,11 +181,11 @@ void Object::_setChangeManager( ObjectCMPtr cm )
             << lunchbox::className( cm ) << std::endl;
     }
 
+    impl_->cm->exit();
     impl_->cm = cm;
     cm->init();
-    LBLOG( LOG_OBJECTS ) << "set new change manager " << lunchbox::className( cm )
-                         << " for " << lunchbox::className( this )
-                         << std::endl;
+    LBLOG( LOG_OBJECTS ) << "set " << lunchbox::className( cm ) << " for "
+                         << lunchbox::className( this ) << std::endl;
 }
 
 ObjectCMPtr Object::_getChangeManager()
@@ -222,7 +222,7 @@ void Object::setupChangeManager( const Object::ChangeType type,
     switch( type )
     {
         case Object::NONE:
-            LBASSERT( !impl_->localNode );
+            LBASSERT( !localNode );
             _setChangeManager( ObjectCM::ZERO );
             break;
 

co/objectCM.cpp

@@ -1,5 +1,5 @@
 
-/* Copyright (c) 2007-2013, Stefan Eilemann <[email protected]>
+/* Copyright (c) 2007-2014, Stefan Eilemann <[email protected]>
  *               2011-2012, Daniel Nachbaur <[email protected]>
  *
  * This file is part of Collage <https://github.com/Eyescale/Collage>
@@ -41,6 +41,15 @@ ObjectCM::ObjectCM( Object* object )
         : _object( object )
 {}
 
+ObjectCM::~ObjectCM()
+{}
+
+void ObjectCM::exit()
+{
+    lunchbox::ScopedFastWrite mutex( _lock );
+    _object = 0;
+}
+
 void ObjectCM::push( const uint128_t& groupID, const uint128_t& typeID,
                      const Nodes& nodes )
 {
@@ -63,9 +72,15 @@ void ObjectCM::push( const uint128_t& groupID, const uint128_t& typeID,
     os.disable(); // handled by remote recv thread
 }
 
-void ObjectCM::sendSync( const MasterCMCommand& command )
+bool ObjectCM::sendSync( const MasterCMCommand& command )
 {
-    LBASSERT( _object );
+    lunchbox::ScopedFastWrite mutex( _lock );
+    if( !_object )
+    {
+        LBWARN << "Sync from detached object requested" << std::endl;
+        return false;
+    }
+
     const uint128_t& maxCachedVersion = command.getMaxCachedVersion();
     const bool useCache =
         command.useCache() &&
@@ -83,10 +98,10 @@ void ObjectCM::sendSync( const MasterCMCommand& command )
     node->send( CMD_NODE_SYNC_OBJECT_REPLY, useCache /*preferMulticast*/ )
         << node->getNodeID() << command.getObjectID() << command.getRequestID()
         << true << command.useCache() << useCache;
-
+    return true;
 }
 
-void ObjectCM::_addSlave( const MasterCMCommand& command,
+bool ObjectCM::_addSlave( const MasterCMCommand& command,
                           const uint128_t& version )
 {
     LBASSERT( version != VERSION_NONE );
@@ -100,15 +115,15 @@ void ObjectCM::_addSlave( const MasterCMCommand& command,
         _sendMapSuccess( command, false /* mc */ );
         _sendEmptyVersion( command, VERSION_NONE, false /* mc */ );
         _sendMapReply( command, VERSION_NONE, true, false, false /* mc */ );
-        return;
+        return true;
     }
 
     const bool replyUseCache = command.useCache() &&
                    (command.getMasterInstanceID() == _object->getInstanceID( ));
-    _initSlave( command, version, replyUseCache );
+    return _initSlave( command, version, replyUseCache );
 }
 
-void ObjectCM::_initSlave( const MasterCMCommand& command,
+bool ObjectCM::_initSlave( const MasterCMCommand& command,
                            const uint128_t& replyVersion, bool replyUseCache )
 {
 #if 0
@@ -133,7 +148,14 @@ void ObjectCM::_initSlave( const MasterCMCommand& command,
 #endif
         _sendMapSuccess( command, false );
         _sendMapReply( command, replyVersion, true, replyUseCache, false );
-        return;
+        return true;
+    }
+
+    lunchbox::ScopedFastWrite mutex( _lock );
+    if( !_object )
+    {
+        LBWARN << "Map to detached object requested" << std::endl;
+        return false;
     }
 
 #ifdef CO_INSTRUMENT_MULTICAST
@@ -154,6 +176,7 @@ void ObjectCM::_initSlave( const MasterCMCommand& command,
         _sendEmptyVersion( command, replyVersion, true /* mc */ );
 
     _sendMapReply( command, replyVersion, true, replyUseCache, true );
+    return true;
 }
 
 void ObjectCM::_sendMapSuccess( const MasterCMCommand& command,