[Bf-blender-cvs] [cc05b661f76] blender2.8: GWN: Fix use after free crash.

Clément Foucault noreply at git.blender.org
Thu Feb 22 12:35:54 CET 2018


Commit: cc05b661f7685ba5b9b046c5fed0cf45ea176d20
Author: Clément Foucault
Date:   Thu Feb 22 12:39:57 2018 +0100
Branches: blender2.8
https://developer.blender.org/rBcc05b661f7685ba5b9b046c5fed0cf45ea176d20

GWN: Fix use after free crash.

This is not an ideal solution but blender freeing system is already well tangled.
So tracking and clearing vao caches when destroying contexts does prevent bad behaviour.

===================================================================

M	intern/gawain/CMakeLists.txt
M	intern/gawain/gawain/gwn_batch.h
A	intern/gawain/gawain/gwn_batch_private.h
M	intern/gawain/src/gwn_batch.c
M	intern/gawain/src/gwn_shader_interface.c
M	intern/gawain/src/gwn_vertex_array_id.cpp
M	source/blender/windowmanager/intern/wm_init_exit.c

===================================================================

diff --git a/intern/gawain/CMakeLists.txt b/intern/gawain/CMakeLists.txt
index 424b364ae8e..177c76327aa 100644
--- a/intern/gawain/CMakeLists.txt
+++ b/intern/gawain/CMakeLists.txt
@@ -23,6 +23,7 @@ set(SRC
 	gawain/gwn_attr_binding.h
 	gawain/gwn_attr_binding_private.h
 	gawain/gwn_batch.h
+	gawain/gwn_batch_private.h
 	gawain/gwn_buffer_id.h
 	gawain/gwn_common.h
 	gawain/gwn_element.h
diff --git a/intern/gawain/gawain/gwn_batch.h b/intern/gawain/gawain/gwn_batch.h
index c676cfef119..4e7f6a15051 100644
--- a/intern/gawain/gawain/gwn_batch.h
+++ b/intern/gawain/gawain/gwn_batch.h
@@ -95,9 +95,6 @@ int GWN_batch_vertbuf_add_ex(Gwn_Batch*, Gwn_VertBuf*, bool own_vbo);
 #define GWN_batch_vertbuf_add(batch, verts) \
 	GWN_batch_vertbuf_add_ex(batch, verts, false)
 
-// This is a private function
-void GWN_batch_remove_interface_ref(Gwn_Batch*, const Gwn_ShaderInterface*);
-
 void GWN_batch_program_set(Gwn_Batch*, GLuint program, const Gwn_ShaderInterface*);
 void GWN_batch_program_unset(Gwn_Batch*);
 // Entire batch draws with one shader program, but can be redrawn later with another program.
diff --git a/intern/gawain/gawain/gwn_batch_private.h b/intern/gawain/gawain/gwn_batch_private.h
new file mode 100644
index 00000000000..6503429c237
--- /dev/null
+++ b/intern/gawain/gawain/gwn_batch_private.h
@@ -0,0 +1,30 @@
+
+// Gawain context
+//
+// This code is part of the Gawain library, with modifications
+// specific to integration with Blender.
+//
+// Copyright 2018 Mike Erwin, Clément Foucault
+//
+// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. If a copy of
+// the MPL was not distributed with this file, You can obtain one at https://mozilla.org/MPL/2.0/.
+
+#pragma once
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include "gwn_batch.h"
+#include "gwn_context.h"
+#include "gwn_shader_interface.h"
+
+void gwn_batch_remove_interface_ref(Gwn_Batch*, const Gwn_ShaderInterface*);
+void gwn_batch_vao_cache_clear(Gwn_Batch*);
+
+void gwn_context_add_batch(Gwn_Context*, Gwn_Batch*);
+void gwn_context_remove_batch(Gwn_Context*, Gwn_Batch*);
+
+#ifdef __cplusplus
+}
+#endif
diff --git a/intern/gawain/src/gwn_batch.c b/intern/gawain/src/gwn_batch.c
index 098c547c662..586112ccc9f 100644
--- a/intern/gawain/src/gwn_batch.c
+++ b/intern/gawain/src/gwn_batch.c
@@ -10,6 +10,7 @@
 // the MPL was not distributed with this file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 #include "gwn_batch.h"
+#include "gwn_batch_private.h"
 #include "gwn_buffer_id.h"
 #include "gwn_vertex_array_id.h"
 #include "gwn_primitive_private.h"
@@ -21,8 +22,11 @@ extern void gpuBindMatrices(const Gwn_ShaderInterface* shaderface);
 
 static void batch_update_program_bindings(Gwn_Batch* batch, unsigned int v_first);
 
-static void Batch_vao_cache_clear(Gwn_Batch* batch)
+void gwn_batch_vao_cache_clear(Gwn_Batch* batch)
 	{
+	if (batch->context == NULL)
+		return;
+
 	if (batch->is_dynamic_vao_count)
 		{
 		for (int i = 0; i < batch->dynamic_vaos.count; ++i)
@@ -52,6 +56,9 @@ static void Batch_vao_cache_clear(Gwn_Batch* batch)
 		batch->static_vaos.vao_ids[i] = 0;
 		batch->static_vaos.interfaces[i] = NULL;
 		}
+
+	gwn_context_remove_batch(batch->context, batch);
+	batch->context = NULL;
 	}
 
 Gwn_Batch* GWN_batch_create_ex(
@@ -116,7 +123,7 @@ void GWN_batch_discard(Gwn_Batch* batch)
 			}
 		}
 
-	Batch_vao_cache_clear(batch);
+	gwn_batch_vao_cache_clear(batch);
 
 	if (batch->free_callback)
 		batch->free_callback(batch, batch->callback_data);
@@ -136,7 +143,7 @@ void GWN_batch_instbuf_set(Gwn_Batch* batch, Gwn_VertBuf* inst, bool own_vbo)
 	assert(inst != NULL);
 #endif
 	// redo the bindings
-	Batch_vao_cache_clear(batch);
+	gwn_batch_vao_cache_clear(batch);
 
 	if (batch->inst != NULL && (batch->owns_flag & GWN_BATCH_OWNS_INSTANCES))
 		GWN_vertbuf_discard(batch->inst);
@@ -153,6 +160,9 @@ int GWN_batch_vertbuf_add_ex(
         Gwn_Batch* batch, Gwn_VertBuf* verts,
         bool own_vbo)
 	{
+	// redo the bindings
+	gwn_batch_vao_cache_clear(batch);
+
 	for (unsigned v = 0; v < GWN_BATCH_VBO_MAX_LEN; ++v)
 		{
 		if (batch->verts[v] == NULL)
@@ -206,7 +216,10 @@ void GWN_batch_program_set(Gwn_Batch* batch, GLuint program, const Gwn_ShaderInt
 	if (batch->vao_id == 0)
 		{
 		if (batch->context == NULL)
+			{
 			batch->context = GWN_context_active_get();
+			gwn_context_add_batch(batch->context, batch);
+			}
 #if TRUST_NO_ONE && 0 // disabled until we use a separate single context for UI.
 		else // Make sure you are not trying to draw this batch in another context.
 			assert(batch->context == GWN_context_active_get());
@@ -282,7 +295,7 @@ void GWN_batch_program_unset(Gwn_Batch* batch)
 	batch->program_in_use = false;
 	}
 
-void GWN_batch_remove_interface_ref(Gwn_Batch* batch, const Gwn_ShaderInterface* interface)
+void gwn_batch_remove_interface_ref(Gwn_Batch* batch, const Gwn_ShaderInterface* interface)
 	{
 	if (batch->is_dynamic_vao_count)
 		{
diff --git a/intern/gawain/src/gwn_shader_interface.c b/intern/gawain/src/gwn_shader_interface.c
index ef3e8f0f3fa..d8103abcd3a 100644
--- a/intern/gawain/src/gwn_shader_interface.c
+++ b/intern/gawain/src/gwn_shader_interface.c
@@ -9,6 +9,7 @@
 // This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. If a copy of
 // the MPL was not distributed with this file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+#include "gwn_batch_private.h"
 #include "gwn_shader_interface.h"
 #include "gwn_vertex_array_id.h"
 #include <stdlib.h>
@@ -282,7 +283,7 @@ void GWN_shaderinterface_discard(Gwn_ShaderInterface* shaderface)
 	// Remove this interface from all linked Batches vao cache.
 	for (int i = 0; i < shaderface->batches_ct; ++i)
 		if (shaderface->batches[i] != NULL)
-			GWN_batch_remove_interface_ref(shaderface->batches[i], shaderface);
+			gwn_batch_remove_interface_ref(shaderface->batches[i], shaderface);
 
 	free(shaderface->batches);
 	// Free memory used by shader interface by its self.
diff --git a/intern/gawain/src/gwn_vertex_array_id.cpp b/intern/gawain/src/gwn_vertex_array_id.cpp
index 1e833f7b2d0..c5611c8f606 100644
--- a/intern/gawain/src/gwn_vertex_array_id.cpp
+++ b/intern/gawain/src/gwn_vertex_array_id.cpp
@@ -9,12 +9,14 @@
 // This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. If a copy of
 // the MPL was not distributed with this file, You can obtain one at https://mozilla.org/MPL/2.0/.#include "buffer_id.h"
 
+#include "gwn_batch_private.h"
 #include "gwn_vertex_array_id.h"
 #include "gwn_context.h"
 #include <vector>
 #include <string.h>
 #include <pthread.h>
 #include <mutex>
+#include <forward_list>
 
 #if TRUST_NO_ONE
 extern "C" {
@@ -30,6 +32,7 @@ static bool thread_is_main()
 
 struct Gwn_Context {
 	GLuint default_vao;
+	std::forward_list<Gwn_Batch*> batches; // Batches that have VAOs from this context
 	std::vector<GLuint> orphaned_vertarray_ids;
 	std::mutex orphans_mutex; // todo: try spinlock instead
 #if TRUST_NO_ONE
@@ -73,8 +76,16 @@ void GWN_context_discard(Gwn_Context* ctx)
 	assert(pthread_equal(pthread_self(), ctx->thread));
 	assert(ctx->orphaned_vertarray_ids.empty());
 #endif
+	// delete remaining vaos
+	while (!ctx->batches.empty())
+		{
+		// this removes the array entry
+		gwn_batch_vao_cache_clear(ctx->batches.front());
+		}
 	glDeleteVertexArrays(1, &ctx->default_vao);
+	(&ctx->orphaned_vertarray_ids)->~vector();
 	(&ctx->orphans_mutex)->~mutex();
+	(&ctx->batches)->~forward_list();
 	free(ctx);
 	active_ctx = NULL;
 	}
@@ -141,3 +152,13 @@ void GWN_vao_free(GLuint vao_id, Gwn_Context* ctx)
 		ctx->orphans_mutex.unlock();
 		}
 	}
+
+void gwn_context_add_batch(Gwn_Context* ctx, Gwn_Batch* batch)
+	{
+	ctx->batches.emplace_front(batch);
+	}
+
+void gwn_context_remove_batch(Gwn_Context* ctx, Gwn_Batch* batch)
+	{
+	ctx->batches.remove(batch);
+	}
diff --git a/source/blender/windowmanager/intern/wm_init_exit.c b/source/blender/windowmanager/intern/wm_init_exit.c
index adb03de4612..43e4f7757f5 100644
--- a/source/blender/windowmanager/intern/wm_init_exit.c
+++ b/source/blender/windowmanager/intern/wm_init_exit.c
@@ -520,6 +520,17 @@ void WM_exit_ext(bContext *C, const bool do_python)
 #ifdef WITH_COMPOSITOR
 	COM_deinitialize();
 #endif
+
+	if (!G.background) {
+#ifdef WITH_OPENSUBDIV
+		BKE_subsurf_osd_cleanup();
+#endif
+
+		GPU_global_buffer_pool_free();
+		GPU_free_unused_buffers();
+
+		GPU_exit();
+	}
 	
 	BKE_blender_free();  /* blender.c, does entire library and spacetypes */
 //	free_matcopybuf();
@@ -565,17 +576,6 @@ void WM_exit_ext(bContext *C, const bool do_python)
 	(void)do_python;
 #endif
 
-	if (!G.background) {
-#ifdef WITH_OPENSUBDIV
-		BKE_subsurf_osd_cleanup();
-#endif
-
-		GPU_global_buffer_pool_free();
-		GPU_free_unused_buffers();
-
-		GPU_exit();
-	}
-
 	BKE_undo_reset();
 	
 	ED_file_exit(); /* for fsmenu */



More information about the Bf-blender-cvs mailing list