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 */

_______________________________________________
Bf-blender-cvs mailing list
Bf-blender-cvs@blender.org
https://lists.blender.org/mailman/listinfo/bf-blender-cvs

Reply via email to