Blender 2.70 test build 1 suddenly closes without errors while opening .blend file #38755

New Issue

Alessandro Sala · 2014-02-22T13:47:48+01:00

Alessandro Sala commented

2014-02-22 13:47:48 +01:00

System Information
OS: Windows 7 Professional 64 bit, Service Pack 1
CPU: Intel Core i7-4700MQ @2.40GHz
RAM: 16 GB
NVidia GeForce GT 750M, driver v332.21

Blender Version
Broken: 2.70 test build 1, Date: 2014-02-20 05:32 Hash: 4789793
Worked: 2.69.0 r60995

Short description of error
When trying to open the attached .blend file, Blender immediately closes without any message.

Exact steps for others to reproduce the error
Run Blender and open the attached .blend file.narghilè.zip

**System Information** OS: Windows 7 Professional 64 bit, Service Pack 1 CPU: Intel Core i7-4700MQ @2.40GHz RAM: 16 GB NVidia GeForce GT 750M, driver v332.21 **Blender Version** Broken: 2.70 test build 1, Date: 2014-02-20 05:32 Hash: 4789793 Worked: 2.69.0 r60995 **Short description of error** When trying to open the attached .blend file, Blender immediately closes without any message. **Exact steps for others to reproduce the error** Run Blender and open the attached .blend file.[narghilè.zip](https://archive.blender.org/developer/F78462/narghilè.zip)

Alessandro Sala commented

2014-02-22 13:47:48 +01:00

Changed status to: 'Open'

Alessandro Sala commented

2014-02-22 13:47:48 +01:00

Added subscriber: @xela-1

Bastien Montagne commented

2014-02-22 15:10:24 +01:00

Added subscriber: @mont29

Bastien Montagne self-assigned this 2014-02-22 15:10:33 +01:00

Bastien Montagne removed their assignment 2014-02-22 15:27:01 +01:00

Sergey Sharybin was assigned by Bastien Montagne

2014-02-22 15:27:01 +01:00

Bastien Montagne commented

2014-02-22 15:27:01 +01:00

Added subscriber: @Sergey

Bastien Montagne commented

2014-02-22 15:27:01 +01:00

Well, another Curve object with a NULL curve_cache (happens in MOD_array.c:380).

Adding a NULL check here is trivial, but not sure this is expected, it smells again like threading issue… @Sergey, thoughts? ;)

Well, another Curve object with a NULL curve_cache (happens in MOD_array.c:380). Adding a NULL check here is trivial, but not sure this is expected, it smells again like threading issue… @Sergey, thoughts? ;)

Lukas Tönne commented

2014-02-23 17:08:45 +01:00

Added subscriber: @LukasTonne

Lukas Tönne commented

2014-02-23 17:08:45 +01:00

I think the problem here might be a dependency cycle: The Tubo dx object has an array modifier making it dependent on the Forma tubo dx curve object, while at the same time Forma tubo dx is a child of Tubo dx:

Dependency cycle detected:
  Forma tubo dx depends on Tubo dx through Parent.
  Tubo dx depends on Forma tubo dx through Array Modifier.

This means the DAG_EVAL_NEED_CURVE_PATH flag doesn't have the desired effect, the curve cache is not updated by the depgraph.
https://developer.blender.org/diffusion/B/browse/master/source/blender/modifiers/intern/MOD_array.c;d92f6b9903c8b914523ef7b3aa228a11f34bd605$128

I think the problem here might be a dependency cycle: The `Tubo dx` object has an array modifier making it dependent on the `Forma tubo dx` curve object, while at the same time `Forma tubo dx` is a child of `Tubo dx`: ``` Dependency cycle detected: Forma tubo dx depends on Tubo dx through Parent. Tubo dx depends on Forma tubo dx through Array Modifier. ``` This means the `DAG_EVAL_NEED_CURVE_PATH` flag doesn't have the desired effect, the curve cache is not updated by the depgraph. https://developer.blender.org/diffusion/B/browse/master/source/blender/modifiers/intern/MOD_array.c;d92f6b9903c8b914523ef7b3aa228a11f34bd605$128

Sergey Sharybin commented

2014-02-24 07:57:18 +01:00

Added subscriber: @brecht

Sergey Sharybin commented

2014-02-24 07:57:18 +01:00

@LukasTonne, sorry for the delay and you're exactly right. But i need to summon @brecht here to the discussion :)

There're three ways to prevent the crash:

Just add NULL-check and do nothing in case of dependency cycle (so we don't evaluate path on-demand and so and allow having regressions in case of cycles)
Add n-demand path evaluation to mimic old behavior. We can put such evaluation inside if(is_main_thread) thread to avoid possible threading conflicts..
Separate curve data into own node in the DAG and evaluate it separately. This seems to solve the issue -- path will be evaluated before other DAG node requests it and evaluation order wouldn't matter in this case.

@brecht, ideally i think #3 is the way to go, but it's the least trivial change. Do you have any suggestions on what to do at this BCon state?

@LukasTonne, sorry for the delay and you're exactly right. But i need to summon @brecht here to the discussion :) There're three ways to prevent the crash: - Just add NULL-check and do nothing in case of dependency cycle (so we don't evaluate path on-demand and so and allow having regressions in case of cycles) - Add n-demand path evaluation to mimic old behavior. We can put such evaluation inside `if(is_main_thread)` thread to avoid possible threading conflicts.. - Separate curve data into own node in the DAG and evaluate it separately. This _seems_ to solve the issue -- path will be evaluated before other DAG node requests it and evaluation order wouldn't matter in this case. @brecht, ideally i think #3 is the way to go, but it's the least trivial change. Do you have any suggestions on what to do at this BCon state?

Lukas Tönne commented

2014-02-24 10:24:39 +01:00

IMO the depgraph should work in a way that it can never leave the scene in a crashable state. I'm not sure how such curve dependencies can be handled properly, perhaps we need a special case hack to ensure valid curve_cache after depgraph eval.

Option 3 would be cleaner, but i have a feeling it doesn't really fit into the depgraph design (however lacking it may be). Do we have any precedence for multiple nodes per object? I guess the new "curve cache node" would never have any dependencies on other nodes itself, so that would prevent any cycles and ensure the condition above.

IMO the depgraph should work in a way that it can never leave the scene in a crashable state. I'm not sure how such curve dependencies can be handled properly, perhaps we need a special case hack to ensure valid curve_cache after depgraph eval. Option 3 would be cleaner, but i have a feeling it doesn't really fit into the depgraph design (however lacking it may be). Do we have any precedence for multiple nodes per object? I guess the new "curve cache node" would never have any dependencies on other nodes itself, so that would prevent any cycles and ensure the condition above.

Brecht Van Lommel commented

2014-02-24 12:39:05 +01:00

Even if we evaluate the curve data as a separate node, it is still possible to have a cyclic dependency. So we need to avoid that kind of crash and add a NULL check whatever the solution?

Option 3 seems like the proper solution as there isn't really a cyclic dependency here in a more find gained dependency graph, but I think it is too late for that in this release. If option 2 is possible without much code/risk it would be good to keep such files working, if not then we should do option 1.

Even if we evaluate the curve data as a separate node, it is still possible to have a cyclic dependency. So we need to avoid that kind of crash and add a NULL check whatever the solution? Option 3 seems like the proper solution as there isn't really a cyclic dependency here in a more find gained dependency graph, but I think it is too late for that in this release. If option 2 is possible without much code/risk it would be good to keep such files working, if not then we should do option 1.

blender-admin commented

2014-02-24 14:15:52 +01:00

This issue was referenced by blender/blender-addons-contrib@1130c53cdb

This issue was referenced by blender/blender-addons-contrib@1130c53cdb24b68d1871a03bad59fad9e6418f29

blender-admin commented

2014-02-24 14:15:52 +01:00

This issue was referenced by 1130c53cdb

This issue was referenced by 1130c53cdb24b68d1871a03bad59fad9e6418f29

Sergey Sharybin commented

2014-02-24 14:16:14 +01:00

Changed status from 'Open' to: 'Resolved'

Sergey Sharybin closed this issue