Browse Source

Bugfix: Outputting managed references in script code now properly issues a GC memory barrier

BearishSun 7 years ago
parent
commit
cec42193fc

+ 5 - 0
Source/BansheeMono/BsMonoUtil.cpp

@@ -160,6 +160,11 @@ namespace bs
 		mono_value_copy(dest, src, klass);
 	}
 
+	void MonoUtil::referenceCopy(void* dest, MonoObject* object)
+	{
+		mono_gc_wbarrier_generic_store(dest, object);
+	}
+
 	bool MonoUtil::isSubClassOf(::MonoClass* subClass, ::MonoClass* parentClass)
 	{
 		return mono_class_is_subclass_of(subClass, parentClass, true) != 0;

+ 7 - 0
Source/BansheeMono/BsMonoUtil.h

@@ -88,6 +88,13 @@ namespace bs
 		 */
 		static void valueCopy(void* dest, void* src, ::MonoClass* klass);
 
+		/**
+		 * Copies the pointer to a reference type @p object to @p dest, ensuring @p dest also points to the object. This
+		 * needs to be used if @p dest is being passed to managed code (e.g. an output parameter in a method). Otherwise
+		 * normal copies can be used. @p dest must be large enough to hold sizeof(MonoObject*).
+		 */
+		static void referenceCopy(void* dest, MonoObject* object);
+
 		/**	Checks if this class is a sub class of the specified class. */
 		static bool isSubClassOf(::MonoClass* subClass, ::MonoClass* parentClass);
 

+ 2 - 0
Source/MBansheeEngine/GUI/GUIElementStateStyle.cs

@@ -2,6 +2,7 @@
 //**************** Copyright (c) 2016 Marko Pintera ([email protected]). All rights reserved. **********************//
 using System;
 using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 
 namespace BansheeEngine
 {
@@ -12,6 +13,7 @@ namespace BansheeEngine
     /// <summary>
     /// Container for texture and text color used in a particular GUI element style.
     /// </summary>
+    [StructLayout(LayoutKind.Sequential)]
     public struct GUIElementStateStyle
     {
         /// <summary>

+ 9 - 3
Source/SBansheeEditor/Wrappers/BsScriptBrowseDialog.cpp

@@ -39,7 +39,7 @@ namespace bs
 				pathArray.set(i, monoString);
 			}
 
-			*outPaths = pathArray.getInternal();
+			MonoUtil::referenceCopy(outPaths, (MonoObject*)pathArray.getInternal());
 			return true;
 		}
 		else
@@ -59,7 +59,10 @@ namespace bs
 		if (EditorUtility::openBrowseDialog(type, defaultFolderNative, "", paths))
 		{
 			if (paths.size() > 0)
-				*outPath = MonoUtil::wstringToMono(paths[0].toWString());
+			{
+				MonoString* path = MonoUtil::wstringToMono(paths[0].toWString());
+				MonoUtil::referenceCopy(outPath, (MonoObject*)path);
+			}
 			else
 				*outPath = nullptr;
 
@@ -83,7 +86,10 @@ namespace bs
 		if (EditorUtility::openBrowseDialog(type, defaultFolderNative, filterListNative, paths))
 		{
 			if (paths.size() > 0)
-				*outPath = MonoUtil::wstringToMono(paths[0].toWString());
+			{
+				MonoString* path = MonoUtil::wstringToMono(paths[0].toWString());
+				MonoUtil::referenceCopy(outPath, (MonoObject*)path);
+			}
 			else
 				*outPath = nullptr;
 

+ 1 - 1
Source/SBansheeEditor/Wrappers/GUI/BsScriptGUIGameObjectField.cpp

@@ -72,7 +72,7 @@ namespace bs
 		GUIGameObjectField* gameObjectField = static_cast<GUIGameObjectField*>(nativeInstance->getGUIElement());
 
 		HGameObject gameObject = gameObjectField->getValue();
-		*output = nativeToManagedGO(gameObject);
+		MonoUtil::referenceCopy(output, nativeToManagedGO(gameObject));
 	}
 
 	void ScriptGUIGameObjectField::internal_setValue(ScriptGUIGameObjectField* nativeInstance, MonoObject* value)

+ 2 - 2
Source/SBansheeEditor/Wrappers/GUI/BsScriptGUIResourceField.cpp

@@ -77,7 +77,7 @@ namespace bs
 		GUIResourceField* resourceField = static_cast<GUIResourceField*>(nativeInstance->getGUIElement());
 
 		HResource resource = resourceField->getValue();
-		*output = nativeToManagedResource(resource);
+		MonoUtil::referenceCopy(output, nativeToManagedResource(resource));
 	}
 
 	void ScriptGUIResourceField::internal_setValue(ScriptGUIResourceField* nativeInstance, MonoObject* value)
@@ -98,7 +98,7 @@ namespace bs
 		GUIResourceField* resourceField = static_cast<GUIResourceField*>(nativeInstance->getGUIElement());
 
 		WeakResourceHandle<Resource> resource = resourceField->getValueWeak();
-		*output = ScriptResourceRef::create(resource);
+		MonoUtil::referenceCopy(output, ScriptResourceRef::create(resource));
 	}
 
 	void ScriptGUIResourceField::internal_setValueRef(ScriptGUIResourceField* nativeInstance, MonoObject* value)

+ 1 - 1
Source/SBansheeEditor/Wrappers/GUI/BsScriptGUITextField.cpp

@@ -71,7 +71,7 @@ namespace bs
 			*output = MonoUtil::wstringToMono(StringUtil::WBLANK);
 
 		GUITextField* field = static_cast<GUITextField*>(nativeInstance->getGUIElement());
-		*output = MonoUtil::wstringToMono(field->getValue());
+		MonoUtil::referenceCopy(output, (MonoObject*)MonoUtil::wstringToMono(field->getValue()));
 	}
 
 	void ScriptGUITextField::internal_setValue(ScriptGUITextField* nativeInstance, MonoString* value)

+ 1 - 1
Source/SBansheeEditor/Wrappers/GUI/BsScriptGUITextureField.cpp

@@ -71,7 +71,7 @@ namespace bs
 		GUITextureField* textureField = static_cast<GUITextureField*>(nativeInstance->getGUIElement());
 
 		HTexture resource = textureField->getValue();
-		*output = nativeToManagedResource(resource);
+		MonoUtil::referenceCopy(output, nativeToManagedResource(resource));
 	}
 
 	void ScriptGUITextureField::internal_setValue(ScriptGUITextureField* nativeInstance, MonoObject* value)

+ 1 - 1
Source/SBansheeEngine/Wrappers/BsScriptHString.cpp

@@ -34,6 +34,6 @@ namespace bs
 
 	void ScriptHString::internal_getValue(ScriptHString* nativeInstance, MonoString** value)
 	{
-		*value = MonoUtil::wstringToMono(nativeInstance->mString.getValue());
+		MonoUtil::referenceCopy(value, (MonoObject*)MonoUtil::wstringToMono(nativeInstance->mString.getValue()));
 	}
 }

+ 17 - 9
Source/SBansheeEngine/Wrappers/GUI/BsScriptGUIElementStyle.cpp

@@ -126,7 +126,7 @@ namespace bs
 		if (style.font != nullptr)
 		{
 			ScriptResourceBase* scriptFont = ScriptResourceManager::instance().getScriptResource(style.font, true);
-			*value = scriptFont->getManagedInstance();
+			MonoUtil::referenceCopy(value, scriptFont->getManagedInstance());
 		}
 		else
 			*value = nullptr;
@@ -195,7 +195,8 @@ namespace bs
 
 	void ScriptGUIElementStyle::internal_GetNormal(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
 	{
-		*value = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.normal);
+		auto temp = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.normal);
+		MonoUtil::valueCopy(value, &temp, ScriptGUIElementStateStyle::getMetaData()->scriptClass->_getInternalClass());
 	}
 
 	void ScriptGUIElementStyle::internal_SetNormal(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
@@ -205,7 +206,8 @@ namespace bs
 
 	void ScriptGUIElementStyle::internal_GetHover(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
 	{
-		*value = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.hover);
+		auto temp = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.hover);
+		MonoUtil::valueCopy(value, &temp, ScriptGUIElementStateStyle::getMetaData()->scriptClass->_getInternalClass());
 	}
 
 	void ScriptGUIElementStyle::internal_SetHover(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
@@ -215,7 +217,8 @@ namespace bs
 
 	void ScriptGUIElementStyle::internal_GetActive(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
 	{
-		*value = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.active);
+		auto temp = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.active);
+		MonoUtil::valueCopy(value, &temp, ScriptGUIElementStateStyle::getMetaData()->scriptClass->_getInternalClass());
 	}
 
 	void ScriptGUIElementStyle::internal_SetActive(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
@@ -225,7 +228,8 @@ namespace bs
 
 	void ScriptGUIElementStyle::internal_GetFocused(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
 	{
-		*value = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.focused);
+		auto temp = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.focused);
+		MonoUtil::valueCopy(value, &temp, ScriptGUIElementStateStyle::getMetaData()->scriptClass->_getInternalClass());
 	}
 
 	void ScriptGUIElementStyle::internal_SetFocused(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
@@ -235,7 +239,8 @@ namespace bs
 
 	void ScriptGUIElementStyle::internal_GetNormalOn(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
 	{
-		*value = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.normalOn);
+		auto temp = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.normalOn);
+		MonoUtil::valueCopy(value, &temp, ScriptGUIElementStateStyle::getMetaData()->scriptClass->_getInternalClass());
 	}
 
 	void ScriptGUIElementStyle::internal_SetNormalOn(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
@@ -245,7 +250,8 @@ namespace bs
 
 	void ScriptGUIElementStyle::internal_GetHoverOn(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
 	{
-		*value = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.hoverOn);
+		auto temp = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.hoverOn);
+		MonoUtil::valueCopy(value, &temp, ScriptGUIElementStateStyle::getMetaData()->scriptClass->_getInternalClass());
 	}
 
 	void ScriptGUIElementStyle::internal_SetHoverOn(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
@@ -255,7 +261,8 @@ namespace bs
 
 	void ScriptGUIElementStyle::internal_GetActiveOn(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
 	{
-		*value = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.activeOn);
+		auto temp = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.activeOn);
+		MonoUtil::valueCopy(value, &temp, ScriptGUIElementStateStyle::getMetaData()->scriptClass->_getInternalClass());
 	}
 
 	void ScriptGUIElementStyle::internal_SetActiveOn(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
@@ -265,7 +272,8 @@ namespace bs
 
 	void ScriptGUIElementStyle::internal_GetFocusedOn(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)
 	{
-		*value = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.focusedOn);
+		auto temp = ScriptGUIElementStateStyle::toManaged(nativeInstance->mElementStyle.focusedOn);
+		MonoUtil::valueCopy(value, &temp, ScriptGUIElementStateStyle::getMetaData()->scriptClass->_getInternalClass());
 	}
 
 	void ScriptGUIElementStyle::internal_SetFocusedOn(ScriptGUIElementStyle* nativeInstance, ScriptGUIElementStateStyleStruct* value)

+ 1 - 1
Source/SBansheeEngine/Wrappers/GUI/BsScriptGUIInputBox.cpp

@@ -52,7 +52,7 @@ namespace bs
 	void ScriptGUIInputBox::internal_getText(ScriptGUIInputBox* nativeInstance, MonoString** text)
 	{
 		GUIInputBox* inputBox = (GUIInputBox*)nativeInstance->getGUIElement();
-		*text = MonoUtil::wstringToMono(inputBox->getText());
+		MonoUtil::referenceCopy(text, (MonoObject*)MonoUtil::wstringToMono(inputBox->getText()));
 	}
 
 	void ScriptGUIInputBox::internal_setText(ScriptGUIInputBox* nativeInstance, MonoString* text)