From 72ee62e0dae84c8077f2b79aa485103d59ce0aad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Tue, 16 Nov 2021 17:18:01 +0100 Subject: Fix crash on freeing hair system Fix a crash when a hair system's `ParticleSettings` ID datablock was linked from another file but couldn't be found. This results in default settings, with `type = PART_EMITTER`, where the particle data still has a non-NULL `hair` pointer. Previously, copies of such a particle system would NOT copy hair data for non-hair particle systems, hence the pointer of the copy pointed to the original data, which got freed (at least) twice upon closing the blend file. This is now fixed by always copying the hair data, regardless of the particle system type. Reviewed by: mont29 Differential Revision: https://developer.blender.org/D13245 --- source/blender/blenkernel/intern/particle.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'source/blender/blenkernel/intern/particle.c') diff --git a/source/blender/blenkernel/intern/particle.c b/source/blender/blenkernel/intern/particle.c index feb997a4c5d..b13fd8a395f 100644 --- a/source/blender/blenkernel/intern/particle.c +++ b/source/blender/blenkernel/intern/particle.c @@ -1148,7 +1148,27 @@ void psys_copy_particles(ParticleSystem *psys_dst, ParticleSystem *psys_src) /* Copy particles and children. */ psys_dst->particles = MEM_dupallocN(psys_src->particles); psys_dst->child = MEM_dupallocN(psys_src->child); - if (psys_dst->part->type == PART_HAIR) { + + /* Ideally this should only be performed if `(psys_dst->part->type == PART_HAIR)`. + * + * But #ParticleData (`psys_dst`) is some sub-data of the #Object ID, while #ParticleSettings + * (`psys_dst->part`) is another ID. In case the particle settings is a linked ID that gets + * missing, it will be replaced (in readfile code) by a place-holder, which defaults to a + * `PART_EMITTER` type of particle settings. + * + * This leads to a situation where each particle of `psys_dst` still has a valid allocated `hair` + * data, which should still be preserved in case the missing particle settings ID becomes valid + * again. + * + * Furthermore, #free_hair() always frees `pa->hair` if it's not NULL, regardless of the + * particle type. So *not* copying here would cause a double free (or more), e.g. freeing the + * copy-on-write copy and the original data will crash Blender. + * In any case, sharing pointers between `psys_src` and `psys_dst` should be forbidden. + * + * So while we could in theory 'sanitize' the situation by setting `pa->hair` to NULL in the new + * copy (in case of non-`PART_HAIR` type), it is probably safer for now to systematically + * duplicate the `hair` data if available. */ + { ParticleData *pa; int p; for (p = 0, pa = psys_dst->particles; p < psys_dst->totpart; p++, pa++) { -- cgit v1.2.3