Page MenuHome

Undo crashes when calling bpy.ops.ed.undo_history(index=0) from Python in background mode
Closed, ResolvedPublic

Description

System Information
Operating System: MacOS mojave 10.14.1, though the same issue has been observed on windows 10 and linux builds
Graphics Card: Intel iris Graphics 6100

Blender Version
Broken:
2.80, 178299278b7a, blender2.8, 2019-01-28

Worked: (optional)

Short description of error
Running operators for undo push and undo history in background causes segfault in 2.80 beta, whereas 2.79b does not suffer the segfault.

Exact steps for others to reproduce the error

On macOS, open a terminal, run blender in background with the python console option with the following commands (also see section below entitled "OUTPUT OF FAILED CALL TO BPY.OPS.ED.UNDO_PUSH")

./blender -b --python-console

Again, referring to the output from the terminal session at the bottom of this post, run the following commands

import bpy

bpy.ops.ed.undo_push()

Observe the output from running this command, the results I got are detailed below (again under "OUTPUT OF FAILED CALL TO BPY.OPS.ED.UNDO_PUSH") showing that the command resulted in a segmentation fault. Running the following sequence on blender 2.79b official release works, also running these same commands works on 2.80.40, a77b63c56943 ( see section below entitled "OUTPUT OF SUCCESSFUL ATTEMPT TO CALL BPY.OPS.ED.UNDO_PUSH")

import bpy

bpy.ops.ed.undo_push()

bpy.ops.ed.undo_history(index=0)

OUTPUT OF SUCCESSFUL ATTEMPT TO CALL BPY.OPS.ED.UNDO_PUSH in 2.80.40, build hash a77b63c56943

jimbosmaccypro:MacOS jamescrowther$ ./blender -b --python-console
Read prefs: /Users/jamescrowther/Library/Application Support/Blender/2.80/config/userpref.blend
found bundled python: /Applications/Blender_exp/2.80.40/blender.app/Contents/Resources/2.80/python
Warning: class DebuggerAddonPreferences contains a properties which should be an annotation!
/Users/jamescrowther/Library/Application Support/Blender/2.80/scripts/addons/remote_debugger.py:116
    make annotation: DebuggerAddonPreferences.eggpath
    make annotation: DebuggerAddonPreferences.pydevpath
Python 3.7.0 (default, Sep 11 2018, 17:36:20) 
[Clang 9.1.0 (clang-902.0.39.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> import bpy
>>> bpy.ops.ed.undo_push
bpy.ops.ed.undo_push(message="Add an undo step *function may be moved*")
>>> bpy.ops.ed.undo_push()
{'FINISHED'}
>>> bpy.ops.ed.undo_history(item=0)
{'FINISHED'}
>>> bpy.app.build_hash
b'a77b63c56943'
>>> bpy.app.version
(2, 80, 40)
>>>

OUTPUT OF FAILED CALL TO BPY.OPS.ED.UNDO_PUSH

jimbosmaccypro:MacOS jamescrowther$ ./blender -b --python-console
Read prefs: /Users/jamescrowther/Library/Application Support/Blender/2.80/config/userpref.blend
found bundled python: /Users/jamescrowther/Downloads/blender-2.80.0-git20190123.178299278b7a-x86_64/blender.app/Contents/Resources/2.80/python
Warning: class DebuggerAddonPreferences contains a properties which should be an annotation!
/Users/jamescrowther/Library/Application Support/Blender/2.80/scripts/addons/remote_debugger.py:116
    make annotation: DebuggerAddonPreferences.eggpath
    make annotation: DebuggerAddonPreferences.pydevpath
Python 3.7.0 (default, Sep 11 2018, 17:36:20) 
[Clang 9.1.0 (clang-902.0.39.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> import bpy
>>> bpy.app.build_hash
b'178299278b7a'
>>> bpy.app.version
(2, 80, 41)
>>> bpy.ops.ed.undo_push()
Writing: /var/folders/x1/p37wzg0s0nn79jjh8_47jllc0000gn/T/blender.crash.txt
Segmentation fault: 11
jimbosmaccypro:MacOS jamescrowther$

Details

Type
Bug

Event Timeline

Philipp Oeser (lichtwerk) triaged this task as Confirmed, Medium priority.Mon, Jan 28, 10:02 AM

Can confirm, bisecting...

Brecht Van Lommel (brecht) raised the priority of this task from Confirmed, Medium to Confirmed, High.Wed, Jan 30, 2:45 PM

@Philipp Oeser (lichtwerk), did you find the commit?

Campbell Barton (campbellbarton) renamed this task from bpy.ops. to Undo crashes when calling bpy.ops.ed.undo_history(index=0) from Python..Thu, Jan 31, 11:07 PM
Campbell Barton (campbellbarton) lowered the priority of this task from Confirmed, High to Normal.
Campbell Barton (campbellbarton) renamed this task from Undo crashes when calling bpy.ops.ed.undo_history(index=0) from Python. to Undo crashes when calling bpy.ops.ed.undo_history(index=0) from Python in background mode.Fri, Feb 1, 3:31 AM

Not using the undo stack in background mode was an intentional change.

While it shouldn't crash this raises the question how much of the undo system should be working in background mode.

Not using undo is nice for lower memory usage for rendering and automation.

We could lazy initialize it so this when operator functions are called, however this is adding another code path for undo to be accessed and the state wont be initialized properly in some cases because some kinds of undo expect an initial memfile undo.

We should either support it fully in background mode, or not at all.

For now I'll disable it because there are many more important todo's in this area.
Longer term we could have a way to enable undo in background mode.

Raise an error instead of crashing, closing rB1b7c361fbb320744e6b7cc0c03b5f850e8fbfd9e

@Campbell Barton (campbellbarton) I can appreciate the reasoning you gave for not reinstating the undo stack in background mode. However, I respectfully request you reconsider. For one, the change that we're talking about was never mentioned in the API changes for 2.80. Yet you mentioned it is an intentional change. That change has now given us a major redesign task to make our addon compatible with 2.80 that we were not expecting and it would be fantastic to have more time to adjust. Given time we can work around.

Furthermore, I ask, what benefit does the community get from this change now? The undo stack has been working in background for the past 4 years we've been using it. If users would have been better off without it all along for extra RAM, surely it would have been targeted long ago?

Please don't misread me, if it is a better solution for blender in the long term to not have undo stack accessible or active for normal operations, then I am all for it. We just need more time to adjust our addon to cope without it.

We've been testing a lot with our users (some 4300 ppl have downloaded the addon we make now) using 2.80. Now we've pretty much got to either stop, or redesign our addon to cope without access to the undo stack. This means that pretty much undo is not supported by our addon.

Since we weren't expecting the change, we've not really begun the design of a workaround yet and it could take a couple of months to do this.

Finally please understand we've been through this before with development changes in Blender that have sparked major re-works. We asked at BCon2016 how we could better access blender's data via the python API to help distribute changes to other nodes for rendering in real time. At that moment, @Sergey Sharybin (sergey) Sharybin mentioned that "we need to do something smart" in relation to how we could get better access to changes made by the user since various parts of the API held pieces of the puzzle of what a user was editing.

Some discussions around 2.80 prior to the first release of the alpha centred on repairing the scene_update_post handlers, which we used extensively. The proposal then from Blender was to make them function as intended and stop abuse of them being used as pseudo event loops (they fired all the time, as opposed to when actual changes to the scene occurred). We welcomed this change on paper. When we got the first version of 2.80, the handlers had been removed, and they've never come back.

We then spent hours redesigning how we'd capture events from blender to synchronise a user's main computer to their render nodes, again in real time. @Campbell Barton (campbellbarton) commented on a question I had regarding doing this and recommended the msgbus system. To this day I hadn't been able to get it to work using the example given on the msgbus system page. I commented about this on the thread that explained how to use it, but no one ever replied to my comment that it wasn't working.

Again, please don't misread this as frustration, I am merely spelling out that we need help here. All I ask is that the functionality that was removed in https://developer.blender.org/rB985712e5ee178789dbdf9124a896252db042ba50 can be restored. I don't believe that they will negatively affect users. As I said above, for the past few years, Blender has operated with the undo stack operating in the background mode and to my knowledge no one complained about this consuming RAM. If people have been shouting about this, please correct me where i am wrong.

So once more, please reconsider restoring the undo stack to how it functioned prior to the changes in https://developer.blender.org/rB985712e5ee178789dbdf9124a896252db042ba50. If there is some important technical reason as to why this can't happen, please explain. I'd love to understand blender better as to how restoring the old way of it working in background would make things worse in 2.80 than in 2.79.

I've respectfully request that this bug be fixed so that the undo system will again be usable in background mode allowing the crowdrender addon to function. Crowdrender is one of the most important addons for our company.

@Jeducious (jameshcrowther), can you briefly explain what you need undo in background mode for? For memory usage on render nodes being able to save memory is quite important, and now might be a good time to make such a breaking change. However there may be solutions to handle both cases.

Note that there is a replacement for scene_update_post in the form of depsgraph_update_post and the new timer API.

Hi @Bastian Neumann (bneu), this is mostly a question of priorities since there are enough high priority undo bugs open which need attention, see: T61045

2.79b also didn't properly initialize undo, eg: wm_files.c

	if (!G.background) {
		BKE_undo_reset();
		BKE_undo_write(C, "original");  /* save current state */
	}

While it didn't crash, it wasn't fully working either.

This is something to look into, if we should have basic memfile undo support in background mode, or add the ability to fully initialize undo.

Some context for how you're using this feature in 2.79 would be helpful.


I commented about this on the thread that explained how to use it, but no one ever replied to my comment that it wasn't working.

Could you link to this post?

@Brecht Van Lommel (brecht) @Campbell Barton (campbellbarton)

Regarding

@Jeducious (jameshcrowther), can you briefly explain what you need undo in background mode for? For memory usage on render nodes being able to save memory is quite important, and now might be a good time to make such a breaking change. However there may be solutions to handle both cases.

Note that there is a replacement for scene_update_post in the form of depsgraph_update_post and the new timer API.

I can gladly explain! :)

Our addon distributes the blend file to remote render nodes and then updates the scene in background mode in response to changes the user makes to their local blender session. We have been using the undo operators to assist with tracking and executing undo/redo when the user does this on their local session. We use this because it is fast. Without the undo/redo operators, we've no way to synchronise the remote nodes with the local node the user is controlling when undo/redo is concerned. This means that we'd have to upload the project file after an undo/redo if the user wants to render.

Since uploading the project is potentially unbounded in time owing to file size/network bandwidth factors, we've always strived to find ways to synchronise changes in real time. This resulted in us using the background process in this fashion since it can run on servers with no graphical interface and is generally leaner on RAM for the same reason. It also can run on computers being used for other things, and since it is a background process, generally doesn't disturb the user on that computer very much (as far as our tests have shown).

As for the depsgraph_update_post.... damn... I wish I'd looked there! I was thinking it would be in bpy.app.handlers!

Also @Campbell Barton (campbellbarton), I will post back, I'll have to search my e-mail for the post/thread I was talking about in relation to the msgbus. And btw, will it still be implemented? I tried the code in the link I gave in my post above, but it still doesn't work for me on 2.80.41

Hi @Bastian Neumann (bneu), this is mostly a question of priorities since there are enough high priority undo bugs open which need attention, see: T61045

2.79b also didn't properly initialize undo, eg: wm_files.c

	if (!G.background) {
		BKE_undo_reset();
		BKE_undo_write(C, "original");  /* save current state */
	}

While it didn't crash, it wasn't fully working either.

This is something to look into, if we should have basic memfile undo support in background mode, or add the ability to fully initialize undo.

Some context for how you're using this feature in 2.79 would be helpful.


I commented about this on the thread that explained how to use it, but no one ever replied to my comment that it wasn't working.

Could you link to this post?

@Campbell Barton (campbellbarton) Yep, here is the link -> P563

I maintain an exporter add-on which messes extensively with the scene during export, then uses undo to clean everything up. Now that undo is unavailable in background mode junk objects are leaking out everywhere. I don't think that many users of my addon are running it in background mode...but *I* am for my unit tests, and those directly affect the quality of the addon.

Trying to keep track of all the changes made to the scene would be inordinately difficult and introduce many bugs, any one of which could do permanent damage user data. Being able to rely on undo is a critical feature. Please re-enable it!

@Jeducious (jameshcrowther), updated P563 although this should be documented.

Would rather avoid using undo for exporters, Blender's bundled exporters take copies of the meshes w/ modifiers applied for eg.
Can so how its handy, but think it's not really good to rely on undo for exporters in general.

I also take copies of objects. I then, among other things:

  • Fiddle with the user's settings
  • Clear unkeyed animation poses
  • Scrub the timeline
  • Remove or disable everything except the object I'm currently writing out

I'm not going to reinvent the wheel when it comes to restoring the scene's original state, I'm going to use Blender's existing state restoration feature! The one which is battle-tested every day by hundreds of thousands of users.

Undo is also helpful because you can fire it from a finally block, ensuring that everything is correctly restored even if your script died halfway through and left things in some indeterminate state.

Campbell Barton (campbellbarton) changed the task status from Archived to Resolved.Fri, Feb 8, 12:21 AM

Committed basic undo system support in background mode: rB3d16a268ee688da25f72a1adb08fdaab454c344d

You'll need to perform an undo push explicitly to initialize undo, afterwards you can use undo to restore the state.

@Campbell Barton (campbellbarton) Thank you :), just downloaded the latest 2.80 beta directly from blender.org and so far, all working well. Thanks again. Can't overstate how grateful we are.... and relieved too. :)

Thank you for providing an fix for that issue.