Browse Source

Reverting game object finalizer fix as another solution is needed

BearishSun 10 years ago
parent
commit
778de71b66

+ 0 - 5
SBansheeEngine/Include/BsScriptComponent.h

@@ -45,11 +45,6 @@ namespace BansheeEngine
 		 */
 		 */
 		virtual MonoObject* _createManagedInstance(bool construct) override;
 		virtual MonoObject* _createManagedInstance(bool construct) override;
 
 
-		/**
-		 * @copydoc	ScriptObjectBase::_onManagedInstanceFinalized
-		 */
-		void _onManagedInstanceFinalized() override;
-
 		/**
 		/**
 		 * @copydoc	ScriptObjectBase::_onManagedInstanceDeleted
 		 * @copydoc	ScriptObjectBase::_onManagedInstanceDeleted
 		 */
 		 */

+ 1 - 2
SBansheeEngine/Include/BsScriptGameObject.h

@@ -33,9 +33,8 @@ namespace BansheeEngine
 		 * @copydoc	ScriptObjectBase::endRefresh
 		 * @copydoc	ScriptObjectBase::endRefresh
 		 */
 		 */
 		virtual void endRefresh(const ScriptObjectBackup& backupData) override;
 		virtual void endRefresh(const ScriptObjectBackup& backupData) override;
-
 	protected:
 	protected:
-		std::atomic<bool> mRefreshInProgress;
+		bool mRefreshInProgress;
 	};
 	};
 
 
 	/**
 	/**

+ 0 - 6
SBansheeEngine/Include/BsScriptGameObjectManager.h

@@ -10,8 +10,6 @@ namespace BansheeEngine
 	 * @brief	Manages all active GameObject interop objects. GameObjects can be created from native
 	 * @brief	Manages all active GameObject interop objects. GameObjects can be created from native
 	 *			code and used in managed code therefore we need to keep a dictionary or all the native
 	 *			code and used in managed code therefore we need to keep a dictionary or all the native
 	 *			objects we have mapped to managed objects.
 	 *			objects we have mapped to managed objects.
-	 *
-	 * @note	Thread safe.
 	 */
 	 */
 	class BS_SCR_BE_EXPORT ScriptGameObjectManager : public Module<ScriptGameObjectManager>
 	class BS_SCR_BE_EXPORT ScriptGameObjectManager : public Module<ScriptGameObjectManager>
 	{
 	{
@@ -94,9 +92,5 @@ namespace BansheeEngine
 
 
 		UnorderedMap<UINT64, ScriptGameObjectEntry> mScriptGameObjects;
 		UnorderedMap<UINT64, ScriptGameObjectEntry> mScriptGameObjects;
 		HEvent mOnAssemblyReloadDoneConn;
 		HEvent mOnAssemblyReloadDoneConn;
-
-		// Mutex needed as we need to be able unregister game objects as soon as they're finalized, and that happens on
-		// a separate finalizer thread. 
-		mutable Mutex mMutex;
 	};
 	};
 }
 }

+ 1 - 8
SBansheeEngine/Include/BsScriptObject.h

@@ -61,14 +61,7 @@ namespace BansheeEngine
 		virtual void _restoreManagedInstance() { }
 		virtual void _restoreManagedInstance() { }
 
 
 		/**
 		/**
-		 * @brief	Called when the managed instance gets finalized. Triggered by the runtime
-		 *			on a finalizer thread.
-		 */
-		virtual void _onManagedInstanceFinalized() { }
-
-		/**
-		 * @brief	Called when the managed instance gets finalized. Triggered on the main thread
-		 *			some time after _onManagedInstanceFinalized is called.
+		 * @brief	Called when the managed instance gets finalized by the CLR.
 		 */
 		 */
 		virtual void _onManagedInstanceDeleted();
 		virtual void _onManagedInstanceDeleted();
 
 

+ 0 - 5
SBansheeEngine/Include/BsScriptSceneObject.h

@@ -40,11 +40,6 @@ namespace BansheeEngine
 
 
 		ScriptSceneObject(MonoObject* instance, const HSceneObject& sceneObject);
 		ScriptSceneObject(MonoObject* instance, const HSceneObject& sceneObject);
 
 
-		/**
-		 * @copydoc	ScriptObjectBase::_onManagedInstanceFinalized
-		 */
-		void _onManagedInstanceFinalized() override;
-
 		/**
 		/**
 		 * @copydoc	ScriptObjectBase::_onManagedInstanceDeleted
 		 * @copydoc	ScriptObjectBase::_onManagedInstanceDeleted
 		 */
 		 */

+ 7 - 23
SBansheeEngine/Source/BsScriptComponent.cpp

@@ -187,37 +187,21 @@ namespace BansheeEngine
 
 
 	void ScriptComponent::endRefresh(const ScriptObjectBackup& backupData)
 	void ScriptComponent::endRefresh(const ScriptObjectBackup& backupData)
 	{
 	{
-		// It's possible that managed component is destroyed but a reference to it
-		// is still kept during assembly refresh. Such components shouldn't be restored
-		// so we delete them.
-		if (mManagedComponent.isDestroyed(true))
-		{
-			ScriptGameObjectManager::instance().destroyScriptGameObject(this);
+		ComponentBackupData componentBackup = any_cast<ComponentBackupData>(backupData.data);
+		mManagedComponent->restore(mManagedInstance, componentBackup, mTypeMissing);
 
 
-			mManagedInstance = nullptr;
-			bs_delete(this);
-		}
-		else
-		{
-			ComponentBackupData componentBackup = any_cast<ComponentBackupData>(backupData.data);
-			mManagedComponent->restore(mManagedInstance, componentBackup, mTypeMissing);
-		}
-		
 		ScriptGameObjectBase::endRefresh(backupData);
 		ScriptGameObjectBase::endRefresh(backupData);
 	}
 	}
 
 
-	void ScriptComponent::_onManagedInstanceFinalized()
-	{
-		if (!mRefreshInProgress.load(std::memory_order_acquire))
-			ScriptGameObjectManager::instance().destroyScriptGameObject(this);
-	}
-
 	void ScriptComponent::_onManagedInstanceDeleted()
 	void ScriptComponent::_onManagedInstanceDeleted()
 	{
 	{
 		mManagedInstance = nullptr;
 		mManagedInstance = nullptr;
 
 
-		if (!mRefreshInProgress.load(std::memory_order_acquire))
-			bs_delete(this);
+		// It's possible that managed component is destroyed but a reference to it
+		// is still kept during assembly refresh. Such components shouldn't be restored
+		// so we delete them.
+		if (!mRefreshInProgress || mManagedComponent.isDestroyed(true))
+			ScriptGameObjectManager::instance().destroyScriptGameObject(this);
 	}
 	}
 
 
 	void ScriptComponent::setNativeHandle(const HGameObject& gameObject)
 	void ScriptComponent::setNativeHandle(const HGameObject& gameObject)

+ 2 - 2
SBansheeEngine/Source/BsScriptGameObject.cpp

@@ -11,14 +11,14 @@ namespace BansheeEngine
 
 
 	ScriptObjectBackup ScriptGameObjectBase::beginRefresh()
 	ScriptObjectBackup ScriptGameObjectBase::beginRefresh()
 	{
 	{
-		mRefreshInProgress.store(true, std::memory_order_release);
+		mRefreshInProgress = true;
 
 
 		return PersistentScriptObjectBase::beginRefresh();
 		return PersistentScriptObjectBase::beginRefresh();
 	}
 	}
 
 
 	void ScriptGameObjectBase::endRefresh(const ScriptObjectBackup& backupData)
 	void ScriptGameObjectBase::endRefresh(const ScriptObjectBackup& backupData)
 	{
 	{
-		mRefreshInProgress.store(false, std::memory_order_release);
+		mRefreshInProgress = false;
 
 
 		PersistentScriptObjectBase::endRefresh(backupData);
 		PersistentScriptObjectBase::endRefresh(backupData);
 	}
 	}

+ 23 - 55
SBansheeEngine/Source/BsScriptGameObjectManager.cpp

@@ -36,19 +36,11 @@ namespace BansheeEngine
 
 
 	ScriptSceneObject* ScriptGameObjectManager::getOrCreateScriptSceneObject(const HSceneObject& sceneObject)
 	ScriptSceneObject* ScriptGameObjectManager::getOrCreateScriptSceneObject(const HSceneObject& sceneObject)
 	{
 	{
-		Lock<> lock(mMutex);
+		ScriptSceneObject* so = getScriptSceneObject(sceneObject);
+		if (so != nullptr)
+			return so;
 
 
-		auto findIter = mScriptGameObjects.find(sceneObject.getInstanceId());
-		if (findIter != mScriptGameObjects.end())
-			return static_cast<ScriptSceneObject*>(findIter->second.instance);
-
-		MonoClass* sceneObjectClass = ScriptAssemblyManager::instance().getSceneObjectClass();
-		MonoObject* instance = sceneObjectClass->createInstance();
-
-		ScriptSceneObject* nativeInstance = new (bs_alloc<ScriptSceneObject>()) ScriptSceneObject(instance, sceneObject);
-		mScriptGameObjects[sceneObject.getInstanceId()] = ScriptGameObjectEntry(nativeInstance, false);
-
-		return nativeInstance;
+		return createScriptSceneObject(sceneObject);
 	}
 	}
 
 
 	ScriptSceneObject* ScriptGameObjectManager::createScriptSceneObject(const HSceneObject& sceneObject)
 	ScriptSceneObject* ScriptGameObjectManager::createScriptSceneObject(const HSceneObject& sceneObject)
@@ -61,11 +53,11 @@ namespace BansheeEngine
 
 
 	ScriptSceneObject* ScriptGameObjectManager::createScriptSceneObject(MonoObject* existingInstance, const HSceneObject& sceneObject)
 	ScriptSceneObject* ScriptGameObjectManager::createScriptSceneObject(MonoObject* existingInstance, const HSceneObject& sceneObject)
 	{
 	{
-		Lock<> lock(mMutex);
-
-		auto findIter = mScriptGameObjects.find(sceneObject.getInstanceId());
-		if(findIter != mScriptGameObjects.end())
-			BS_EXCEPT(InvalidStateException, "Script SceneObject for this SceneObject already exists.");
+		ScriptGameObjectBase* go = getScriptGameObject(sceneObject.getInstanceId());
+		if (go != nullptr)
+		{
+			BS_EXCEPT(InvalidStateException, "Script component for this SceneObject already exists.");
+		}
 
 
 		ScriptSceneObject* nativeInstance = new (bs_alloc<ScriptSceneObject>()) ScriptSceneObject(existingInstance, sceneObject);
 		ScriptSceneObject* nativeInstance = new (bs_alloc<ScriptSceneObject>()) ScriptSceneObject(existingInstance, sceneObject);
 		mScriptGameObjects[sceneObject.getInstanceId()] = ScriptGameObjectEntry(nativeInstance, false);
 		mScriptGameObjects[sceneObject.getInstanceId()] = ScriptGameObjectEntry(nativeInstance, false);
@@ -75,11 +67,11 @@ namespace BansheeEngine
 
 
 	ScriptComponent* ScriptGameObjectManager::createScriptComponent(MonoObject* existingInstance, const GameObjectHandle<ManagedComponent>& component)
 	ScriptComponent* ScriptGameObjectManager::createScriptComponent(MonoObject* existingInstance, const GameObjectHandle<ManagedComponent>& component)
 	{
 	{
-		Lock<> lock(mMutex);
-
-		auto findIter = mScriptGameObjects.find(component->getInstanceId());
-		if(findIter != mScriptGameObjects.end())
+		ScriptGameObjectBase* go = getScriptGameObject(component.getInstanceId());
+		if (go != nullptr)
+		{
 			BS_EXCEPT(InvalidStateException, "Script component for this Component already exists.");
 			BS_EXCEPT(InvalidStateException, "Script component for this Component already exists.");
+		}
 
 
 		ScriptComponent* nativeInstance = new (bs_alloc<ScriptComponent>()) ScriptComponent(existingInstance);
 		ScriptComponent* nativeInstance = new (bs_alloc<ScriptComponent>()) ScriptComponent(existingInstance);
 		nativeInstance->setNativeHandle(component);
 		nativeInstance->setNativeHandle(component);
@@ -90,41 +82,24 @@ namespace BansheeEngine
 
 
 	ScriptComponent* ScriptGameObjectManager::getScriptComponent(const GameObjectHandle<ManagedComponent>& component) const
 	ScriptComponent* ScriptGameObjectManager::getScriptComponent(const GameObjectHandle<ManagedComponent>& component) const
 	{
 	{
-		Lock<> lock(mMutex);
-
-		auto findIter = mScriptGameObjects.find(component.getInstanceId());
-		if(findIter != mScriptGameObjects.end())
-			return static_cast<ScriptComponent*>(findIter->second.instance);
-
-		return nullptr;
+		ScriptGameObjectBase* go = getScriptGameObject(component.getInstanceId());
+		return static_cast<ScriptComponent*>(go);
 	}
 	}
 
 
 	ScriptComponent* ScriptGameObjectManager::getScriptComponent(UINT64 instanceId) const
 	ScriptComponent* ScriptGameObjectManager::getScriptComponent(UINT64 instanceId) const
 	{
 	{
-		Lock<> lock(mMutex);
-
-		auto findIter = mScriptGameObjects.find(instanceId);
-		if (findIter != mScriptGameObjects.end())
-			return static_cast<ScriptComponent*>(findIter->second.instance);
-
-		return nullptr;
+		ScriptGameObjectBase* go = getScriptGameObject(instanceId);
+		return static_cast<ScriptComponent*>(go);
 	}
 	}
 
 
 	ScriptSceneObject* ScriptGameObjectManager::getScriptSceneObject(const HSceneObject& sceneObject) const
 	ScriptSceneObject* ScriptGameObjectManager::getScriptSceneObject(const HSceneObject& sceneObject) const
 	{
 	{
-		Lock<> lock(mMutex);
-
-		auto findIter = mScriptGameObjects.find(sceneObject.getInstanceId());
-		if(findIter != mScriptGameObjects.end())
-			return static_cast<ScriptSceneObject*>(findIter->second.instance);
-
-		return nullptr;
+		ScriptGameObjectBase* go = getScriptGameObject(sceneObject.getInstanceId());
+		return static_cast<ScriptSceneObject*>(go);
 	}
 	}
 
 
 	ScriptGameObjectBase* ScriptGameObjectManager::getScriptGameObject(UINT64 instanceId) const
 	ScriptGameObjectBase* ScriptGameObjectManager::getScriptGameObject(UINT64 instanceId) const
 	{
 	{
-		Lock<> lock(mMutex);
-
 		auto findIter = mScriptGameObjects.find(instanceId);
 		auto findIter = mScriptGameObjects.find(instanceId);
 		if (findIter != mScriptGameObjects.end())
 		if (findIter != mScriptGameObjects.end())
 			return static_cast<ScriptSceneObject*>(findIter->second.instance);
 			return static_cast<ScriptSceneObject*>(findIter->second.instance);
@@ -134,22 +109,15 @@ namespace BansheeEngine
 
 
 	void ScriptGameObjectManager::destroyScriptGameObject(ScriptGameObjectBase* gameObject)
 	void ScriptGameObjectManager::destroyScriptGameObject(ScriptGameObjectBase* gameObject)
 	{
 	{
-		Lock<> lock(mMutex);
+		UINT64 instanceId = gameObject->getNativeHandle().getInstanceId();
+		mScriptGameObjects.erase(instanceId);
 
 
-		UINT64 idx = gameObject->getNativeHandle().getInstanceId();
-		mScriptGameObjects.erase(idx);
+		bs_delete(gameObject);
 	}
 	}
 
 
 	void ScriptGameObjectManager::sendComponentResetEvents()
 	void ScriptGameObjectManager::sendComponentResetEvents()
 	{
 	{
-		UnorderedMap<UINT64, ScriptGameObjectEntry> scriptGameObjects;
-
-		{
-			Lock<> lock(mMutex);
-			scriptGameObjects = mScriptGameObjects;
-		}
-
-		for (auto& scriptObjectEntry : scriptGameObjects)
+		for (auto& scriptObjectEntry : mScriptGameObjects)
 		{
 		{
 			const ScriptGameObjectEntry& entry = scriptObjectEntry.second;
 			const ScriptGameObjectEntry& entry = scriptObjectEntry.second;
 
 

+ 0 - 2
SBansheeEngine/Source/BsScriptObjectManager.cpp

@@ -75,8 +75,6 @@ namespace BansheeEngine
 	{
 	{
 		assert(instance != nullptr);
 		assert(instance != nullptr);
 
 
-		instance->_onManagedInstanceFinalized();
-
 		BS_LOCK_MUTEX(mMutex);
 		BS_LOCK_MUTEX(mMutex);
 		mFinalizedObjects[mFinalizedQueueIdx].push_back(instance);
 		mFinalizedObjects[mFinalizedQueueIdx].push_back(instance);
 	}
 	}

+ 2 - 8
SBansheeEngine/Source/BsScriptSceneObject.cpp

@@ -338,18 +338,12 @@ namespace BansheeEngine
 		return false;
 		return false;
 	}
 	}
 
 
-	void ScriptSceneObject::_onManagedInstanceFinalized()
-	{
-		if (!mRefreshInProgress.load(std::memory_order_acquire))
-			ScriptGameObjectManager::instance().destroyScriptGameObject(this);
-	}
-
 	void ScriptSceneObject::_onManagedInstanceDeleted()
 	void ScriptSceneObject::_onManagedInstanceDeleted()
 	{
 	{
 		mManagedInstance = nullptr;
 		mManagedInstance = nullptr;
 
 
-		if (!mRefreshInProgress.load(std::memory_order_acquire))
-			bs_delete(this);
+		if (!mRefreshInProgress)
+			ScriptGameObjectManager::instance().destroyScriptGameObject(this);
 	}
 	}
 
 
 	void ScriptSceneObject::setNativeHandle(const HGameObject& gameObject)
 	void ScriptSceneObject::setNativeHandle(const HGameObject& gameObject)