فهرست منبع

Fixed a crash when re-opening the scene window due to finalizer incorrectly cleaning up the active window
Added delayed finalization to fix issue with gizmo manager where it would cause a crash when called from non-sim thread

Marko Pintera 10 سال پیش
والد
کامیت
b59b26a228

+ 0 - 4
SBansheeEditor/Source/BsScriptEditorWindow.cpp

@@ -98,11 +98,7 @@ namespace BansheeEngine
 	void ScriptEditorWindow::_onManagedInstanceDeleted()
 	void ScriptEditorWindow::_onManagedInstanceDeleted()
 	{
 	{
 		if (!mRefreshInProgress)
 		if (!mRefreshInProgress)
-		{
-			unregisterScriptEditorWindow(mName);
-
 			ScriptObject::_onManagedInstanceDeleted();
 			ScriptObject::_onManagedInstanceDeleted();
-		}
 		else
 		else
 			mManagedInstance = nullptr;
 			mManagedInstance = nullptr;
 	}
 	}

+ 22 - 0
SBansheeEngine/Include/BsScriptObjectManager.h

@@ -15,6 +15,25 @@ namespace BansheeEngine
 
 
 		void refreshAssemblies();
 		void refreshAssemblies();
 
 
+		void update();
+
+		/**
+		 * @brief	Call this when object finalizer is triggered. At the end of the frame
+		 *			all objects queued with this method will have their _onManagedInstanceDeleted methods
+		 *			triggered.
+		 *
+		 * @note	Thread safe.
+		 */
+		void notifyObjectFinalized(ScriptObjectBase* instance);
+
+		/**
+		 * @brief	Triggers _onManagedInstanceDeleted deleted callbacks on all objects that were finalized this frame.
+		 *			This allows the native portions of those objects to properly clean up any resources.
+		 *
+		 * @note	Sim thread.
+		 */
+		void processFinalizedObjects();
+
 		/**
 		/**
 		 * @brief	Triggered right after a domain was reloaded. This signals the outside world that they should
 		 * @brief	Triggered right after a domain was reloaded. This signals the outside world that they should
 		 *			update any kept Mono references as the old ones will no longer be valid.
 		 *			update any kept Mono references as the old ones will no longer be valid.
@@ -25,5 +44,8 @@ namespace BansheeEngine
 		Event<void()> onRefreshComplete;
 		Event<void()> onRefreshComplete;
 	private:
 	private:
 		Set<ScriptObjectBase*> mScriptObjects;
 		Set<ScriptObjectBase*> mScriptObjects;
+
+		Vector<ScriptObjectBase*> mFinalizedObjects[2];
+		std::atomic<UINT32> mFinalizedQueueIdx;
 	};
 	};
 }
 }

+ 5 - 0
SBansheeEngine/Source/BsScriptEnginePlugin.cpp

@@ -43,6 +43,11 @@ namespace BansheeEngine
 		return nullptr;
 		return nullptr;
 	}
 	}
 
 
+	extern "C" BS_SCR_BE_EXPORT void updatePlugin()
+	{
+		ScriptObjectManager::instance().update();
+	}
+
 	extern "C" BS_SCR_BE_EXPORT void unloadPlugin()
 	extern "C" BS_SCR_BE_EXPORT void unloadPlugin()
 	{
 	{
 		ScriptVirtualInput::shutDown();
 		ScriptVirtualInput::shutDown();

+ 3 - 1
SBansheeEngine/Source/BsScriptObjectImpl.cpp

@@ -17,6 +17,8 @@ namespace BansheeEngine
 
 
 	void ScriptObjectImpl::internal_managedInstanceDeleted(ScriptObjectBase* instance)
 	void ScriptObjectImpl::internal_managedInstanceDeleted(ScriptObjectBase* instance)
 	{
 	{
-		instance->_onManagedInstanceDeleted();
+		// This method gets called on the finalizer thread, but so that we don't need to deal
+		// with multi-threading issues we just delay it and execute it on the sim thread.
+		ScriptObjectManager::instance().notifyObjectFinalized(instance);
 	}
 	}
 }
 }

+ 25 - 0
SBansheeEngine/Source/BsScriptObjectManager.cpp

@@ -6,6 +6,7 @@
 namespace BansheeEngine
 namespace BansheeEngine
 {
 {
 	ScriptObjectManager::ScriptObjectManager()
 	ScriptObjectManager::ScriptObjectManager()
+		:mFinalizedQueueIdx(0)
 	{
 	{
 
 
 	}
 	}
@@ -30,6 +31,9 @@ namespace BansheeEngine
 			backupData[scriptObject] = scriptObject->beginRefresh();
 			backupData[scriptObject] = scriptObject->beginRefresh();
 
 
 		MonoManager::instance().unloadScriptDomain();
 		MonoManager::instance().unloadScriptDomain();
+		// Unload script domain should trigger finalizers on everything, but since we usually delay
+		// their processing we need to manually trigger it here.
+		processFinalizedObjects();
 
 
 		for (auto& scriptObject : mScriptObjects)
 		for (auto& scriptObject : mScriptObjects)
 			assert(scriptObject->isPersistent() && "Non-persistent ScriptObject alive after domain unload.");
 			assert(scriptObject->isPersistent() && "Non-persistent ScriptObject alive after domain unload.");
@@ -50,4 +54,25 @@ namespace BansheeEngine
 
 
 		onRefreshComplete();
 		onRefreshComplete();
 	}
 	}
+
+	void ScriptObjectManager::notifyObjectFinalized(ScriptObjectBase* instance)
+	{
+		UINT32 idx = mFinalizedQueueIdx.load(std::memory_order_relaxed);
+		mFinalizedObjects[idx].push_back(instance);
+	}
+
+	void ScriptObjectManager::update()
+	{
+		processFinalizedObjects();
+	}
+
+	void ScriptObjectManager::processFinalizedObjects()
+	{
+		UINT32 readQueueIdx = mFinalizedQueueIdx.fetch_xor(0x1, std::memory_order_relaxed);
+
+		for (auto& finalizedObj : mFinalizedObjects[readQueueIdx])
+			finalizedObj->_onManagedInstanceDeleted();
+
+		mFinalizedObjects[readQueueIdx].clear();
+	}
 }
 }

+ 1 - 1
TODO.txt

@@ -41,7 +41,7 @@ Add "dirty object" system to C#. Each ScriptResource and ScriptGameObject should
 TODO - Later - When building level for release make sure to clear all prefab diffs (possibly store them elsewhere in the first place)
 TODO - Later - When building level for release make sure to clear all prefab diffs (possibly store them elsewhere in the first place)
        - And all prefab instances should have updateFromPrefab called on them.
        - And all prefab instances should have updateFromPrefab called on them.
 
 
- Test (likely later once I have more editor functionality working):h
+ Test (likely later once I have more editor functionality working):
   - If level save/load works
   - If level save/load works
   - If drag and drop works, both ways, plus double-click to load level
   - If drag and drop works, both ways, plus double-click to load level
   - Game object handle compare
   - Game object handle compare