diff options
author | Jacques Lucke <jacques@blender.org> | 2021-05-26 15:19:01 +0300 |
---|---|---|
committer | Jacques Lucke <jacques@blender.org> | 2021-05-26 15:19:01 +0300 |
commit | b67423f80663990f972f4317d38b8e7662b9e8eb (patch) | |
tree | 7b682f45fbfdb96517d14632ce594c108ebcf87a | |
parent | afec66c024dc2b75447537d45406c06342ec201e (diff) |
Nodes: fix threading issues with node ui storage
Calling BKE_nodetree_attribute_hint_add from multiple threads still
was not safe before..
One issue was that context_map embedded its values directly. So
when context_map grows, all NodeUIStorage would move as well.
I could patch around that by using std::unique_ptr in a few places,
but that just becomes too complex for now.
Instead I simplified the locking a bit by adding just locking a mutex
in NodeTreeUIStorage all the time while an attribute hint is added.
Differential Revision: https://developer.blender.org/D11399
-rw-r--r-- | source/blender/blenkernel/BKE_node_ui_storage.hh | 10 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/node_ui_storage.cc | 27 |
2 files changed, 14 insertions, 23 deletions
diff --git a/source/blender/blenkernel/BKE_node_ui_storage.hh b/source/blender/blenkernel/BKE_node_ui_storage.hh index 8bf89cd8f58..4ec165aad8c 100644 --- a/source/blender/blenkernel/BKE_node_ui_storage.hh +++ b/source/blender/blenkernel/BKE_node_ui_storage.hh @@ -95,21 +95,13 @@ struct AvailableAttributeInfo { }; struct NodeUIStorage { - std::mutex mutex; blender::Vector<NodeWarning> warnings; blender::Set<AvailableAttributeInfo> attribute_hints; - - NodeUIStorage() = default; - /* Needed because the mutex can't be moved or copied. */ - NodeUIStorage(NodeUIStorage &&other) - : warnings(std::move(other.warnings)), attribute_hints(std::move(other.attribute_hints)) - { - } }; struct NodeTreeUIStorage { + std::mutex mutex; blender::Map<NodeTreeEvaluationContext, blender::Map<std::string, NodeUIStorage>> context_map; - std::mutex context_map_mutex; /** * Attribute search uses this to store the fake info for the string typed into a node, in order diff --git a/source/blender/blenkernel/intern/node_ui_storage.cc b/source/blender/blenkernel/intern/node_ui_storage.cc index 7a28fd295fb..e5e9f00c7c3 100644 --- a/source/blender/blenkernel/intern/node_ui_storage.cc +++ b/source/blender/blenkernel/intern/node_ui_storage.cc @@ -39,7 +39,7 @@ using blender::Vector; * bNodeTree struct in DNA. This could change if the node tree had a runtime struct. */ static std::mutex global_ui_storage_mutex; -static void ui_storage_ensure(bNodeTree &ntree) +static NodeTreeUIStorage &ui_storage_ensure(bNodeTree &ntree) { /* As an optimization, only acquire a lock if the UI storage doesn't exist, * because it only needs to be allocated once for every node tree. */ @@ -50,6 +50,7 @@ static void ui_storage_ensure(bNodeTree &ntree) ntree.ui_storage = new NodeTreeUIStorage(); } } + return *ntree.ui_storage; } const NodeUIStorage *BKE_node_tree_ui_storage_get_from_context(const bContext *C, @@ -90,7 +91,7 @@ void BKE_nodetree_ui_storage_free_for_context(bNodeTree &ntree, { NodeTreeUIStorage *ui_storage = ntree.ui_storage; if (ui_storage != nullptr) { - std::lock_guard<std::mutex> lock(ui_storage->context_map_mutex); + std::lock_guard<std::mutex> lock(ui_storage->mutex); ui_storage->context_map.remove(context); } } @@ -126,20 +127,14 @@ static void node_error_message_log(bNodeTree &ntree, } } -static NodeUIStorage &node_ui_storage_ensure(bNodeTree &ntree, +static NodeUIStorage &node_ui_storage_ensure(NodeTreeUIStorage &locked_ui_storage, const NodeTreeEvaluationContext &context, const bNode &node) { - ui_storage_ensure(ntree); - NodeTreeUIStorage &ui_storage = *ntree.ui_storage; - - std::lock_guard<std::mutex> lock(ui_storage.context_map_mutex); Map<std::string, NodeUIStorage> &node_tree_ui_storage = - ui_storage.context_map.lookup_or_add_default(context); - + locked_ui_storage.context_map.lookup_or_add_default(context); NodeUIStorage &node_ui_storage = node_tree_ui_storage.lookup_or_add_default_as( StringRef(node.name)); - return node_ui_storage; } @@ -149,10 +144,12 @@ void BKE_nodetree_error_message_add(bNodeTree &ntree, const NodeWarningType type, std::string message) { + NodeTreeUIStorage &ui_storage = ui_storage_ensure(ntree); + std::lock_guard lock{ui_storage.mutex}; + node_error_message_log(ntree, node, message, type); - NodeUIStorage &node_ui_storage = node_ui_storage_ensure(ntree, context, node); - std::lock_guard lock{node_ui_storage.mutex}; + NodeUIStorage &node_ui_storage = node_ui_storage_ensure(ui_storage, context, node); node_ui_storage.warnings.append({type, std::move(message)}); } @@ -163,8 +160,10 @@ void BKE_nodetree_attribute_hint_add(bNodeTree &ntree, const AttributeDomain domain, const CustomDataType data_type) { - NodeUIStorage &node_ui_storage = node_ui_storage_ensure(ntree, context, node); - std::lock_guard lock{node_ui_storage.mutex}; + NodeTreeUIStorage &ui_storage = ui_storage_ensure(ntree); + std::lock_guard lock{ui_storage.mutex}; + + NodeUIStorage &node_ui_storage = node_ui_storage_ensure(ui_storage, context, node); node_ui_storage.attribute_hints.add_as( AvailableAttributeInfo{attribute_name, domain, data_type}); } |