Page MenuHome

Crash when appending object using operator called from template icon view, followed by undo
Closed, InvalidPublic

Description

System Information
Operating system: Linux-4.15.0-112-generic-x86_64-with-debian-buster-sid 64 Bits
Graphics card: GeForce GTX 1050/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 435.21

Blender Version
Broken: version: 2.83.5, branch: master, commit date: 2020-08-19 06:07, hash: rBc2b144df395f, 2.90 is also affected

Short description of error
I have an operator that appends an object from another file. Calling it and undoing right after, works as expected unless that same operator is called from an update function of an enum, storing preview collection items, as is done for template icon views. Blender will crash on 2.83.5 and 2.90.

video demo

Exact steps for others to reproduce the error

  • Extract the above zip file and open append_obj_undo_crash_from_template_icon_view.blend
  • Run the script
  • In the 3d view sidebar, a new panel 'Append Object' appears under the Tool tab
  • In the Append Object panel, click on the thumbnail, and again on the blender icon inside
    • the append object op was called and a cube was appended to the scene
  • Undo
    • Blender should crash!!!
  • Start Blender again and run the script again
  • This time, run the append op using the button below the thumbnail, then undo again
    • Blender should not crash this time

Related Objects

Event Timeline

MACHIN3 (MACHIN3) updated the task description. (Show Details)

Exactly the same behaviour in

System Information
Operating system: Windows-10-10.0.19041-SP0 64 Bits
Graphics card: GeForce GTX 1080/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 452.06

Blender Version
Broken: version: 2.90.0 Beta, branch: master, commit date: 2020-08-25 16:00, hash: rB21cb6f09ffa8

Confirm crash.

System Information
Operating system: Windows-10-10.0.19041-SP0 64 Bits
Graphics card: GeForce RTX 2070 Super / NVIDIA Corporation 4.5.0 NVIDIA 452.06

Blender Version
2.83.5, branch: master, commit date: 2020-08-19 06:07, hash: rBc2b144df395f

@Robert Guetzkow (rjg) Shouldn't this be BF Blender (2.90) too? It's confirmed two times to crash 2.90.

@MACHIN3 (MACHIN3) Phabricator doesn't let me tag more than one version. I can add both tags in the UI but one is discarded when I save the edit. Therefore, I tagged to lowest version since any fix for 2.83 will always have to be checked for inclusion into the current release as well.

MACHIN3 (MACHIN3) added a comment.EditedMon, Aug 31, 12:42 PM

Understood, thanks!

Robert Guetzkow (rjg) changed the task status from Needs Triage to Confirmed.EditedMon, Aug 31, 11:38 PM

This can be easily reproduced with the provided instructions. base->object in BKE_scene_object_base_flag_sync_from_base was NULL.

BKE_scene_object_base_flag_sync_from_base(Base * base) Line 1779	C
BKE_scene_set_background(Main * bmain, Scene * scene) Line 990	C
setup_app_data(bContext * C, BlendFileData * bfd, const unsigned char * filepath, const BlendFileReadParams * params, ReportList * reports) Line 384	C
setup_app_blend_file_data(bContext * C, BlendFileData * bfd, const unsigned char * filepath, const BlendFileReadParams * params, ReportList * reports) Line 414	C
BKE_blendfile_read_from_memfile(bContext * C, MemFile * memfile, const BlendFileReadParams * params, ReportList * reports) Line 511	C
BKE_memfile_undo_decode(MemFileUndoData * mfu, const int undo_direction, const bool use_old_bmain_data, bContext * C) Line 87	C
memfile_undosys_step_decode(bContext * C, Main * bmain, UndoStep * us_p, int undo_direction, bool UNUSED_is_final) Line 192	C
undosys_step_decode(bContext * C, Main * bmain, UndoStack * ustack, UndoStep * us, int dir, bool is_final) Line 211	C
BKE_undosys_step_undo_with_data_ex(UndoStack * ustack, bContext * C, UndoStep * us, bool use_skip) Line 695	C
BKE_undosys_step_undo_with_data(UndoStack * ustack, bContext * C, UndoStep * us) Line 723	C
BKE_undosys_step_undo(UndoStack * ustack, bContext * C) Line 728	C
ed_undo_step_impl(bContext * C, int step, const unsigned char * undoname, int undo_index, ReportList * reports) Line 208	C
ed_undo_step_direction(bContext * C, int step, ReportList * reports) Line 272	C
ed_undo_exec(bContext * C, wmOperator * op) Line 407	C
wm_operator_invoke(bContext * C, wmOperatorType * ot, wmEvent * event, PointerRNA * properties, ReportList * reports, const bool poll_only, bool use_last_properties) Line 1312	C
wm_handler_operator_call(bContext * C, ListBase * handlers, wmEventHandler * handler_base, wmEvent * event, PointerRNA * properties, const unsigned char * kmi_idname) Line 2131	C
wm_handlers_do_keymap_with_keymap_handler(bContext * C, wmEvent * event, ListBase * handlers, wmEventHandler_Keymap * handler, wmKeyMap * keymap, const bool do_debug_handler) Line 2441	C
wm_handlers_do_intern(bContext * C, wmEvent * event, ListBase * handlers) Line 2738	C
wm_handlers_do(bContext * C, wmEvent * event, ListBase * handlers) Line 2865	C
wm_event_do_handlers(bContext * C) Line 3398	C
WM_main(bContext * C) Line 485	C
main(int argc, const unsigned char * * UNUSED_argv_c) Line 548	C
Bastien Montagne (mont29) claimed this task.

Answer is very simple: don't do this. Never ever call an operator from a property update function, this is begging for all kind of issues. Just don't.

@Bastien Montagne (mont29) Perhaps this should be added to the Gotchas page, if it's a known limitation?

@Robert Guetzkow (rjg) Good point, this page needs some updates for undo/redo section too actually.

Since this is crashing Blender, and since it never was was problem before 2.83, I feel like this warants more than "don't do this".

New undo code comes with more complexity and responsibility (old one, by nuking and reloading the whole blend file, was safer, since it was not possible to get invalid pointers out of it, if stored undo step was OK). Calling an operator from an update callback has never been recommended though, those callbacks are supposed to remain very fast and efficient, and as localized as possible. Especially messing with the Main database has always been rather bad idea, fact that it has been working previously was mere luck, definitively not by design.

Understood, thanks for clarifying!