Crash during access to dupli weights #49273

New Issue

Alexander Romanov · 2016-09-06T15:36:32+02:00

Alexander Romanov commented

2016-09-06 15:36:32 +02:00

Short description of error

The crash is happening only when you invoke the attached script by passing it to Blender through command line.

Exact steps for others to reproduce the error

Extract files from zip, cd to the directory
Run: blender -P test_script.py training.blend

=> Crash

If you will run similar script when Blender is being already loaded, there is no crash:

import bpy

for dw in bpy.data.objects['Circle'].modifiers['ParticleSystem 1'].particle_system.settings.dupli_weights:
    print(dw.name)

dupli_weights_crash.zip

I suppose that the old pointer is not updated.

**Short description of error** The crash is happening only when you invoke the attached script by passing it to Blender through command line. **Exact steps for others to reproduce the error** - Extract files from zip, cd to the directory - Run: blender -P test_script.py training.blend # => Crash If you will run similar script when Blender is being already loaded, there is no crash: ``` import bpy for dw in bpy.data.objects['Circle'].modifiers['ParticleSystem 1'].particle_system.settings.dupli_weights: print(dw.name) ``` [dupli_weights_crash.zip](https://archive.blender.org/developer/F354397/dupli_weights_crash.zip) I suppose that the old pointer is not updated.

Alexander Romanov commented

2016-09-06 15:36:32 +02:00

Changed status to: 'Open'

Alexander Romanov commented

2016-09-06 15:36:32 +02:00

Added subscriber: @AlexanderRomanov

Bastien Montagne commented

2016-09-06 16:22:52 +02:00

Added subscriber: @mont29

Bastien Montagne self-assigned this 2016-09-06 16:22:52 +02:00

Bastien Montagne commented

2016-09-06 16:22:52 +02:00

Something seems to be going bad in readfile.c's lib_link_particlesettings(), not sure why code here is so much intricated :/

Something seems to be going bad in readfile.c's `lib_link_particlesettings()`, not sure why code here is so much intricated :/

Bastien Montagne commented

2016-09-06 18:29:00 +02:00

Yuck… issue is actually very hairy. :(

Problem is, lib_link_particlesettings() is assuming its dup_group Group is already fully valid (i.e. its object pointers are valid), which is not the case if dup_group is linked from a library not yet processed. That, and the fact that object assigned to dw->ob are not considered as “real reference”, i.e. it’s not enough to consider said object as directly linked, afaict.

I’m terribly tempted to just NULL-ify those dw->ob pointers, and let later code reset them to correct value, when valid indices are available… Means you'll get 'No Object' from your script at startup, but won't crash, and particle code seems to nicely re-populate those pointers later (probably on update).

Yuck… issue is actually *very* hairy. :( Problem is, `lib_link_particlesettings()` is assuming its dup_group Group is already fully valid (i.e. its object pointers are valid), which is not the case if dup_group is linked from a library not yet processed. That, and the fact that object assigned to dw->ob are not considered as “real reference”, i.e. it’s not enough to consider said object as directly linked, afaict. I’m terribly tempted to just NULL-ify those dw->ob pointers, and let later code reset them to correct value, when valid indices are available… Means you'll get 'No Object' from your script at startup, but won't crash, and particle code seems to nicely re-populate those pointers later (probably on update).

Bastien Montagne commented

2016-09-06 21:57:00 +02:00

uuur… but setting dw->ob to NULL forces the system to recreate dummy dupliweights with all same '1' value :(

this needs further investigations…

uuur… but setting dw->ob to NULL forces the system to recreate dummy dupliweights with all same '1' value :( this needs further investigations…

Alexander Romanov commented

2016-09-07 11:23:36 +02:00

Added subscribers: @AlexKowel, @EvgenyRodygin, @yurikovelenov

Bastien Montagne commented

2016-09-07 12:04:11 +02:00

Added subscriber: @Sergey

Bastien Montagne commented

2016-09-07 12:04:11 +02:00

OK, have a patch fixing this, would not mind @Sergey having an eye on it before I commit it though…

P391: #49273

diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index 57d499f..5a230ca 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -4058,6 +4058,24 @@ static void lib_link_partdeflect(FileData *fd, ID *id, PartDeflect *pd)
 		pd->f_source = newlibadr(fd, id->lib, pd->f_source);
 }
 
+static bool lib_link_particlesettings_dweight_index_is_ok(const ParticleSettings *part)
+{
+	/* Check for old files without indices (all indices set to 0 by DNA read code). */
+
+	/* Special case for only one object in the group. */
+	if (BLI_listbase_is_single(&part->dupliweights)) {
+		return true;
+	}
+
+	for (ParticleDupliWeight *dw = part->dupliweights.first; dw; dw = dw->next) {
+		if (dw->index > 0) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
 static void lib_link_particlesettings(FileData *fd, Main *main)
 {
 	ParticleSettings *part;
@@ -4087,30 +4105,17 @@ static void lib_link_particlesettings(FileData *fd, Main *main)
 			}
 
 			if (part->dupliweights.first && part->dup_group) {
-				int index_ok = 0;
-				/* check for old files without indices (all indexes 0) */
-				if (BLI_listbase_is_single(&part->dupliweights)) {
-					/* special case for only one object in the group */
-					index_ok = 1;
-				}
-				else {
-					for (dw = part->dupliweights.first; dw; dw = dw->next) {
-						if (dw->index > 0) {
-							index_ok = 1;
-							break;
-						}
-					}
-				}
-
-				if (index_ok) {
-					/* if we have indexes, let's use them */
-					for (dw = part->dupliweights.first; dw; dw = dw->next) {
-						GroupObject *go = (GroupObject *)BLI_findlink(&part->dup_group->gobject, dw->index);
-						dw->ob = go ? go->ob : NULL;
-					}
+				if (lib_link_particlesettings_dweight_index_is_ok(part)) {
+					/* Do nothing here, we have to search for group objects in another separated step.
+					 * Reason is, the used group may be linked from another library, which has not yet
+					 * been 'lib_linked'.
+					 * Since dw->ob is not considered as an object user (it does not make objet directly linked),
+					 * we may have no valid way to retrieve it yet.
+					 * See #49273. */
 				}
 				else {
-					/* otherwise try to get objects from own library (won't work on library linked groups) */
+					/* No index of objects in doupli_group (old .blend files), we try to get objects directly
+					 * from own library (won't work on library linked groups) */
 					for (dw = part->dupliweights.first; dw; dw = dw->next) {
 						dw->ob = newlibadr(fd, part->id.lib, dw->ob);
 					}
@@ -4158,6 +4163,21 @@ static void lib_link_particlesettings(FileData *fd, Main *main)
 	}
 }
 
+static void lib_link_particlesettings_post(Main *main)
+{
+	ParticleSettings *part;
+	ParticleDupliWeight *dw;
+
+	for (part = main->particle.first; part; part = part->id.next) {
+		if (part->dupliweights.first && part->dup_group && lib_link_particlesettings_dweight_index_is_ok(part)) {
+			for (dw = part->dupliweights.first; dw; dw = dw->next) {
+				GroupObject *go = (GroupObject *)BLI_findlink(&part->dup_group->gobject, dw->index);
+				dw->ob = go ? go->ob : NULL;
+			}
+		}
+	}
+}
+
 static void direct_link_partdeflect(PartDeflect *pd)
 {
 	if (pd) pd->rng = NULL;
@@ -10376,9 +10396,17 @@ static void read_libraries(FileData *basefd, ListBase *mainlist)
 		if (mainptr->curlib->filedata)
 			lib_link_all(mainptr->curlib->filedata, mainptr);
 		
-		if (mainptr->curlib->filedata) blo_freefiledata(mainptr->curlib->filedata);
+		if (mainptr->curlib->filedata)
+			blo_freefiledata(mainptr->curlib->filedata);
 		mainptr->curlib->filedata = NULL;
 	}
+
+	/* Extra hack needed by particles' dupliweight, see lib_link_particlesettings() for details. */
+	for (mainptr = mainl->next; mainptr; mainptr = mainptr->next) {
+		if (mainptr->particle.first) {
+			lib_link_particlesettings_post(mainptr);
+		}
+	}
 }

OK, have a patch fixing this, would not mind @Sergey having an eye on it before I commit it though… [P391: #49273](https://archive.blender.org/developer/P391.txt) ``` diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c index 57d499f..5a230ca 100644 --- a/source/blender/blenloader/intern/readfile.c +++ b/source/blender/blenloader/intern/readfile.c @@ -4058,6 +4058,24 @@ static void lib_link_partdeflect(FileData *fd, ID *id, PartDeflect *pd) pd->f_source = newlibadr(fd, id->lib, pd->f_source); } +static bool lib_link_particlesettings_dweight_index_is_ok(const ParticleSettings *part) +{ + /* Check for old files without indices (all indices set to 0 by DNA read code). */ + + /* Special case for only one object in the group. */ + if (BLI_listbase_is_single(&part->dupliweights)) { + return true; + } + + for (ParticleDupliWeight *dw = part->dupliweights.first; dw; dw = dw->next) { + if (dw->index > 0) { + return true; + } + } + + return false; +} + static void lib_link_particlesettings(FileData *fd, Main *main) { ParticleSettings *part; @@ -4087,30 +4105,17 @@ static void lib_link_particlesettings(FileData *fd, Main *main) } if (part->dupliweights.first && part->dup_group) { - int index_ok = 0; - /* check for old files without indices (all indexes 0) */ - if (BLI_listbase_is_single(&part->dupliweights)) { - /* special case for only one object in the group */ - index_ok = 1; - } - else { - for (dw = part->dupliweights.first; dw; dw = dw->next) { - if (dw->index > 0) { - index_ok = 1; - break; - } - } - } - - if (index_ok) { - /* if we have indexes, let's use them */ - for (dw = part->dupliweights.first; dw; dw = dw->next) { - GroupObject *go = (GroupObject *)BLI_findlink(&part->dup_group->gobject, dw->index); - dw->ob = go ? go->ob : NULL; - } + if (lib_link_particlesettings_dweight_index_is_ok(part)) { + /* Do nothing here, we have to search for group objects in another separated step. + * Reason is, the used group may be linked from another library, which has not yet + * been 'lib_linked'. + * Since dw->ob is not considered as an object user (it does not make objet directly linked), + * we may have no valid way to retrieve it yet. + * See #49273. */ } else { - /* otherwise try to get objects from own library (won't work on library linked groups) */ + /* No index of objects in doupli_group (old .blend files), we try to get objects directly + * from own library (won't work on library linked groups) */ for (dw = part->dupliweights.first; dw; dw = dw->next) { dw->ob = newlibadr(fd, part->id.lib, dw->ob); } @@ -4158,6 +4163,21 @@ static void lib_link_particlesettings(FileData *fd, Main *main) } } +static void lib_link_particlesettings_post(Main *main) +{ + ParticleSettings *part; + ParticleDupliWeight *dw; + + for (part = main->particle.first; part; part = part->id.next) { + if (part->dupliweights.first && part->dup_group && lib_link_particlesettings_dweight_index_is_ok(part)) { + for (dw = part->dupliweights.first; dw; dw = dw->next) { + GroupObject *go = (GroupObject *)BLI_findlink(&part->dup_group->gobject, dw->index); + dw->ob = go ? go->ob : NULL; + } + } + } +} + static void direct_link_partdeflect(PartDeflect *pd) { if (pd) pd->rng = NULL; @@ -10376,9 +10396,17 @@ static void read_libraries(FileData *basefd, ListBase *mainlist) if (mainptr->curlib->filedata) lib_link_all(mainptr->curlib->filedata, mainptr); - if (mainptr->curlib->filedata) blo_freefiledata(mainptr->curlib->filedata); + if (mainptr->curlib->filedata) + blo_freefiledata(mainptr->curlib->filedata); mainptr->curlib->filedata = NULL; } + + /* Extra hack needed by particles' dupliweight, see lib_link_particlesettings() for details. */ + for (mainptr = mainl->next; mainptr; mainptr = mainptr->next) { + if (mainptr->particle.first) { + lib_link_particlesettings_post(mainptr); + } + } } ```

Bastien Montagne commented

2016-09-07 12:45:54 +02:00

OK, so after discussing it with Sergey we decided it was not a good idea to hack in readcode, especially not for something that is doomed to complete rewrite, and even worse, for something that is purely runtime caching helper.

So will instead slightly modify 'update' code of particles' duplis to restore correct dw->ob pointers. This means you won't have correct values for dw.name until you update the scene.

OK, so after discussing it with Sergey we decided it was not a good idea to hack in readcode, especially not for something that is doomed to complete rewrite, and even worse, for something that is purely runtime caching helper. So will instead slightly modify 'update' code of particles' duplis to restore correct `dw->ob` pointers. This means you won't have correct values for `dw.name` until you update the scene.

blender-admin commented

2016-09-07 12:45:58 +02:00

This issue was referenced by bcc863993a

This issue was referenced by bcc863993adfe019454d2da014528ac922fffd41

Bastien Montagne commented

2016-09-07 12:52:27 +02:00

Changed status from 'Open' to: 'Resolved'

Bastien Montagne closed this issue

2016-09-07 12:52:27 +02:00

Alexander Romanov commented

2016-09-07 13:35:50 +02:00

Thanks! It's better then crash!

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Crash during access to dupli weights #49273

=> Crash