Prechádzať zdrojové kódy

Remove virtual call in destructor in cases where the render interface was destroyed before the call to Rml::Shutdown, see #222. Update the lifetime requirements for render interface, and add an assert in the render interface destructor to identify cases where this requirement is not respected.

Michael Ragazzon 4 rokov pred
rodič
commit
e04a8e3e21

+ 2 - 1
Include/RmlUi/Core/Core.h

@@ -96,7 +96,8 @@ RMLUICORE_API FontEngineInterface* GetFontEngineInterface();
 /// @param[in] name The new name of the context. This must be unique.
 /// @param[in] dimensions The initial dimensions of the new context.
 /// @param[in] render_interface The custom render interface to use, or nullptr to use the default.
-/// @lifetime If specified, the render interface must be kept alive until after the context is destroyed or the call to Rml::Shutdown.
+/// @lifetime If specified, the render interface must be kept alive until after the call to Rml::Shutdown. Alternatively, the render interface can be
+///           destroyed after all contexts it belongs to have been destroyed and a subsequent call has been made to Rml::ReleaseTextures.
 /// @return A non-owning pointer to the new context, or nullptr if the context could not be created.
 RMLUICORE_API Context* CreateContext(const String& name, Vector2i dimensions, RenderInterface* render_interface = nullptr);
 /// Removes and destroys a context.

+ 6 - 1
Source/Core/RenderInterface.cpp

@@ -38,7 +38,12 @@ RenderInterface::RenderInterface()
 
 RenderInterface::~RenderInterface()
 {
-	TextureDatabase::ReleaseTextures(this);
+	// Note: We cannot automatically release the textures from the database here, because that involves a virtual call to this interface during its
+	// destruction which is illegal.
+	RMLUI_ASSERTMSG(!TextureDatabase::HoldsReferenceToRenderInterface(this),
+		"RenderInterface is being destroyed, but there are still active textures referencing it in the texture database. Ensure either that (1) the "
+		"render interface is destroyed *after* the call to Rml::Shutdown, or that (2) all the contexts the render interface belongs to has been "
+		"destroyed and a subsequent call has been made to Rml::ReleaseTextures before the render interface is destroyed.");
 }
 
 // Called by RmlUi when it wants to compile geometry it believes will be static for the forseeable future.

+ 23 - 11
Source/Core/TextureDatabase.cpp

@@ -15,7 +15,7 @@
  *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -27,10 +27,10 @@
  */
 
 #include "TextureDatabase.h"
-#include "TextureResource.h"
 #include "../../Include/RmlUi/Core/Core.h"
 #include "../../Include/RmlUi/Core/StringUtilities.h"
 #include "../../Include/RmlUi/Core/SystemInterface.h"
+#include "TextureResource.h"
 
 namespace Rml {
 
@@ -49,7 +49,7 @@ TextureDatabase::~TextureDatabase()
 #ifdef RMLUI_DEBUG
 	// All textures not owned by the database should have been released at this point.
 	int num_leaks_file = 0;
-	
+
 	for (auto& texture : textures)
 		num_leaks_file += (texture.second.use_count() > 1);
 
@@ -58,8 +58,8 @@ TextureDatabase::~TextureDatabase()
 
 	if (total_num_leaks > 0)
 	{
-		Log::Message(Log::LT_ERROR, "Textures leaked during shutdown. Total: %d  From file: %d  Generated: %d.",
-			total_num_leaks, num_leaks_file, num_leaks_callback);
+		Log::Message(Log::LT_ERROR, "Textures leaked during shutdown. Total: %d  From file: %d  Generated: %d.", total_num_leaks, num_leaks_file,
+			num_leaks_callback);
 	}
 #endif
 
@@ -76,8 +76,6 @@ void TextureDatabase::Shutdown()
 	delete texture_database;
 }
 
-// If the requested texture is already in the database, it will be returned with an extra reference count. If not, it
-// will be loaded through the application's render interface.
 SharedPtr<TextureResource> TextureDatabase::Fetch(const String& source, const String& source_directory)
 {
 	String path;
@@ -86,11 +84,9 @@ SharedPtr<TextureResource> TextureDatabase::Fetch(const String& source, const St
 	else
 		GetSystemInterface()->JoinPath(path, StringUtilities::Replace(source_directory, '|', ':'), source);
 
-	TextureMap::iterator iterator = texture_database->textures.find(path);
+	auto iterator = texture_database->textures.find(path);
 	if (iterator != texture_database->textures.end())
-	{
 		return iterator->second;
-	}
 
 	auto resource = MakeShared<TextureResource>();
 	resource->Set(path);
@@ -114,7 +110,7 @@ void TextureDatabase::RemoveCallbackTexture(TextureResource* texture)
 StringList TextureDatabase::GetSourceList()
 {
 	StringList result;
-	
+
 	if (texture_database)
 	{
 		result.reserve(texture_database->textures.size());
@@ -138,4 +134,20 @@ void TextureDatabase::ReleaseTextures(RenderInterface* render_interface)
 	}
 }
 
+bool TextureDatabase::HoldsReferenceToRenderInterface(RenderInterface* render_interface)
+{
+	if (texture_database)
+	{
+		for (const auto& texture : texture_database->textures)
+			if (texture.second->HoldsRenderInterface(render_interface))
+				return true;
+
+		for (const auto& texture : texture_database->callback_textures)
+			if (texture->HoldsRenderInterface(render_interface))
+				return true;
+	}
+
+	return false;
+}
+
 } // namespace Rml

+ 17 - 15
Source/Core/TextureDatabase.h

@@ -15,7 +15,7 @@
  *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -37,42 +37,44 @@ class RenderInterface;
 class TextureResource;
 
 /**
-	@author Peter Curry
+    @author Peter Curry
  */
 
-class TextureDatabase
-{
+class TextureDatabase {
 public:
 	static void Initialise();
 	static void Shutdown();
 
-    /// Fetch a texture resource from file.
-	/// If the requested texture is already in the database, it will be returned with an extra
-	/// reference count. If not, it will be loaded through the application's render interface.
+	/// Fetch a texture resource from file.
+	/// The texture will be returned from the database if it already exists, otherwise a new
+	/// entry will be added and returned.
 	static SharedPtr<TextureResource> Fetch(const String& source, const String& source_directory);
 
 	/// Release all textures bound through a render interface.
-    /// Pass nullptr to release all textures in the database.
+	/// Pass nullptr to release all textures in the database.
 	static void ReleaseTextures(RenderInterface* render_interface = nullptr);
 
-    /// Adds a texture resource with a callback function and stores it as a weak (raw) pointer in the database.
-    static void AddCallbackTexture(TextureResource* texture);
+	/// Adds a texture resource with a callback function and stores it as a weak (raw) pointer in the database.
+	static void AddCallbackTexture(TextureResource* texture);
 
-    /// Removes a callback texture from the database.
-    static void RemoveCallbackTexture(TextureResource* texture);
+	/// Removes a callback texture from the database.
+	static void RemoveCallbackTexture(TextureResource* texture);
 
 	/// Return a list of all texture sources currently in the database.
 	static StringList GetSourceList();
 
+	/// For debugging. Returns true if any textures hold a reference to the given render interface.
+	static bool HoldsReferenceToRenderInterface(RenderInterface* render_interface);
+
 private:
 	TextureDatabase();
 	~TextureDatabase();
 
-	using TextureMap = UnorderedMap< String, SharedPtr<TextureResource> >;
+	using TextureMap = UnorderedMap<String, SharedPtr<TextureResource>>;
 	TextureMap textures;
 
-    using CallbackTextureMap = UnorderedSet< TextureResource* >;
-    CallbackTextureMap callback_textures;
+	using CallbackTextureMap = UnorderedSet<TextureResource*>;
+	CallbackTextureMap callback_textures;
 };
 
 } // namespace Rml

+ 1 - 1
Source/Core/TextureResource.cpp

@@ -118,7 +118,7 @@ void TextureResource::Release(RenderInterface* render_interface)
 	}
 	else
 	{
-		TextureDataMap::iterator texture_iterator = texture_data.find(render_interface);
+		auto texture_iterator = texture_data.find(render_interface);
 		if (texture_iterator == texture_data.end())
 			return;
 

+ 11 - 9
Source/Core/TextureResource.h

@@ -15,7 +15,7 @@
  *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -29,20 +29,19 @@
 #ifndef RMLUI_CORE_TEXTURERESOURCE_H
 #define RMLUI_CORE_TEXTURERESOURCE_H
 
-#include "../../Include/RmlUi/Core/Traits.h"
 #include "../../Include/RmlUi/Core/Texture.h"
+#include "../../Include/RmlUi/Core/Traits.h"
 
 namespace Rml {
 
 /**
-	A texture resource stores application-generated texture data (handle and dimensions) for each
-	unique render interface that needs to render the data. It is used through a Texture object.
+    A texture resource stores application-generated texture data (handle and dimensions) for each
+    unique render interface that needs to render the data. It is used through a Texture object.
 
-	@author Peter Curry
+    @author Peter Curry
  */
 
-class TextureResource : public NonCopyMoveable
-{
+class TextureResource : public NonCopyMoveable {
 public:
 	TextureResource();
 	~TextureResource();
@@ -66,6 +65,9 @@ public:
 	/// Releases the texture's handle.
 	void Release(RenderInterface* render_interface = nullptr);
 
+	/// For debugging. Returns true if the texture holds a reference to the given render interface, otherwise false.
+	inline bool HoldsRenderInterface(RenderInterface* render_interface) const { return texture_data.count(render_interface); }
+
 private:
 	void Reset();
 
@@ -74,8 +76,8 @@ private:
 
 	String source;
 
-	using TextureData = Pair< TextureHandle, Vector2i >;
-	using TextureDataMap = SmallUnorderedMap< RenderInterface*, TextureData >;
+	using TextureData = Pair<TextureHandle, Vector2i>;
+	using TextureDataMap = SmallUnorderedMap<RenderInterface*, TextureData>;
 	TextureDataMap texture_data;
 
 	UniquePtr<TextureCallback> texture_callback;