From 38688adaad4ed0d8b2f3fc8117e6a5c9b8b64e92 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Fri, 17 Mar 2023 11:17:01 +0100 Subject: [PATCH] Fix #105818: material preview invalid memory access reported by ASAN Preview render depsgraphs are put in the depsgraph registry concurrently with other threads. This was lacking a mutex lock and a map value that remains unchanged when other elements of the map are updated. Pull Request: https://projects.blender.org/blender/blender/pulls/105839 --- .../depsgraph/intern/depsgraph_registry.cc | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/source/blender/depsgraph/intern/depsgraph_registry.cc b/source/blender/depsgraph/intern/depsgraph_registry.cc index 96336167b5a..dea87550542 100644 --- a/source/blender/depsgraph/intern/depsgraph_registry.cc +++ b/source/blender/depsgraph/intern/depsgraph_registry.cc @@ -5,6 +5,9 @@ * \ingroup depsgraph */ +#include +#include + #include "intern/depsgraph_registry.h" #include "BLI_utildefines.h" @@ -13,7 +16,19 @@ namespace blender::deg { -using GraphRegistry = Map
>; +/* Global registry for dependency graphs associated with a main database. + * + * Threads may add or remove depsgraphs for different mains concurrently + * (for example for preview rendering), but not the same main. */ + +/* Use pointer for map value to ensure span returned by get_all_registered_graphs + * remains unchanged as other mains are added or removed. */ +typedef std::unique_ptr> GraphSetPtr; +struct GraphRegistry { + Map
map; + std::mutex mutex; +}; + static GraphRegistry &get_graph_registry() { static GraphRegistry graph_registry; @@ -22,28 +37,37 @@ static GraphRegistry &get_graph_registry() void register_graph(Depsgraph *depsgraph) { + GraphRegistry &graph_registry = get_graph_registry(); Main *bmain = depsgraph->bmain; - get_graph_registry().lookup_or_add_default(bmain).add_new(depsgraph); + + std::lock_guard lock{graph_registry.mutex}; + graph_registry.map + .lookup_or_add_cb(bmain, []() { return std::make_unique>(); }) + ->add_new(depsgraph); } void unregister_graph(Depsgraph *depsgraph) { Main *bmain = depsgraph->bmain; GraphRegistry &graph_registry = get_graph_registry(); - VectorSet &graphs = graph_registry.lookup(bmain); - graphs.remove(depsgraph); + + std::lock_guard lock{graph_registry.mutex}; + GraphSetPtr &graphs = graph_registry.map.lookup(bmain); + graphs->remove(depsgraph); /* If this was the last depsgraph associated with the main, remove the main entry as well. */ - if (graphs.is_empty()) { - graph_registry.remove(bmain); + if (graphs->is_empty()) { + graph_registry.map.remove(bmain); } } Span get_all_registered_graphs(Main *bmain) { - VectorSet *graphs = get_graph_registry().lookup_ptr(bmain); - if (graphs != nullptr) { - return *graphs; + GraphRegistry &graph_registry = get_graph_registry(); + std::lock_guard lock{graph_registry.mutex}; + GraphSetPtr *graphs = graph_registry.map.lookup_ptr(bmain); + if (graphs) { + return **graphs; } return {}; }