From 317f09ebf990ab4a5d033bab61a0aa8816772615 Mon Sep 17 00:00:00 2001 From: Bastien Montagne Date: Wed, 28 Jul 2021 17:36:40 +0200 Subject: Fix T90170: `RNA_property_pointer_get` creating data in non-thread-safe way. Protect this accessor with a local static mutex when it needs to create/write data. Ideally accessors should never create or modify data, but there are some cases where this bad behavior is currently unavoidable. This is the case of the Pointer accessor when the actual IDProperty has not yet been created. NOTE: this fixes a memory leak in liboverride diffing process when several different overrides use a same linked reference ID. Differential Revision: https://developer.blender.org/D12060 --- source/blender/makesrna/intern/rna_access.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'source/blender/makesrna') diff --git a/source/blender/makesrna/intern/rna_access.c b/source/blender/makesrna/intern/rna_access.c index 46d60bf0da9..c991216da11 100644 --- a/source/blender/makesrna/intern/rna_access.c +++ b/source/blender/makesrna/intern/rna_access.c @@ -36,6 +36,7 @@ #include "BLI_dynstr.h" #include "BLI_ghash.h" #include "BLI_math.h" +#include "BLI_threads.h" #include "BLI_utildefines.h" #include "BLF_api.h" @@ -3676,6 +3677,8 @@ PointerRNA RNA_property_pointer_get(PointerRNA *ptr, PropertyRNA *prop) PointerPropertyRNA *pprop = (PointerPropertyRNA *)prop; IDProperty *idprop; + static ThreadMutex lock = BLI_MUTEX_INITIALIZER; + BLI_assert(RNA_property_type(prop) == PROP_POINTER); if ((idprop = rna_idproperty_check(&prop, ptr))) { @@ -3695,9 +3698,14 @@ PointerRNA RNA_property_pointer_get(PointerRNA *ptr, PropertyRNA *prop) return pprop->get(ptr); } if (prop->flag & PROP_IDPROPERTY) { - /* XXX temporary hack to add it automatically, reading should - * never do any write ops, to ensure thread safety etc. */ + /* NOTE: While creating/writing data in an accessor is really bad design-wise, this is + * currently very difficult to avoid in that case. So a global mutex is used to keep ensuring + * thread safety. */ + BLI_mutex_lock(&lock); + /* NOTE: We do not need to check again for existence of the pointer after locking here, since + * this is also done in #RNA_property_pointer_add itself. */ RNA_property_pointer_add(ptr, prop); + BLI_mutex_unlock(&lock); return RNA_property_pointer_get(ptr, prop); } return PointerRNA_NULL; -- cgit v1.2.3