Maniphest T74067

Crash: Custom Panel UI Toggle while Undoing
Confirmed, NormalBUG
Actions

Assigned To
Campbell Barton (campbellbarton)
Authored By
Wayde Moss (GuiltyGhost)
Feb 21 2020, 10:09 AM
Tags
  • BF Blender
  • User Interface
Subscribers
Bastien Montagne (mont29)
Campbell Barton (campbellbarton)
Julian Eisel (Severin)
Oliver Weissbarth (oweissbarth)
Wayde Moss (GuiltyGhost)

Description

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

Blender Version
Broken: version: 2.83 (sub 4), branch: master, commit date: 2020-02-21 01:21, hash: rBb56957f0af0e
Broken: 2.82
Broken: 2.81

Short description of error

Read Access Violation when toggling custom UI button and repeatedly undoing. Crash unlikely to happen first time but will happen eventually. In the video, I toggle a few times, then hold Undo while toggling. (Video shows exact steps)

Sometimes it can take a bit of time before the crash happens. It might happen faster if you reload the file then repeat the toggling process.

Exact steps for others to reproduce the error

Open attached .blend File
Run Script
Go to 3DView -> N -> Panel "Crash"
Toggle the button a few times
Hold Undo while continuing to toggle the button.
Crash occurs eventually.

crash_simpler1.blend535 KBDownload

Below is a pic of where the exception is thrown (Blender version for source pic attached)



Thank you for your time and hard work.

Revisions and Commits

rB Blender
Needs ReviewD7795 Fix T74067: Crash when the UI accesses stale data

Related Objects

Event Timeline

Wayde Moss (GuiltyGhost) created this task.Feb 21 2020, 10:09 AM
Richard Antalik (ISS) changed the task status from Needs Triage to Confirmed.Feb 21 2020, 3:51 PM
Richard Antalik (ISS) added projects: Add-ons (BF-Blender), Python API.
Bastien Montagne (mont29) changed the subtype of this task from "Report" to "Bug".Feb 24 2020, 11:49 AM
Bastien Montagne (mont29) edited projects, added User Interface; removed Python API, Add-ons (BF-Blender).
Bastien Montagne (mont29) added subscribers: Campbell Barton (campbellbarton), Bastien Montagne (mont29).

Can reproduce as well, from the backtrace below it's clearly that UI (either editors data, or from the event handling, but I think in that case it's the button' RNA pointer itself) is still using old data from before the undo step, after undo has been performed.

==6478==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d0008601b8 at pc 0x55e952c278ae bp 0x7ffc55aa5800 sp 0x7ffc55aa57f8
READ of size 1 at 0x60d0008601b8 thread T0
    #0 0x55e952c278ad in rna_idproperty_find /home/guest/blender/src/source/blender/makesrna/intern/rna_access.c:401
    #1 0x55e952c28c90 in rna_idproperty_check_ex /home/guest/blender/src/source/blender/makesrna/intern/rna_access.c:576
    #2 0x55e952c29209 in rna_idproperty_check /home/guest/blender/src/source/blender/makesrna/intern/rna_access.c:610
    #3 0x55e952c38c37 in RNA_property_boolean_get /home/guest/blender/src/source/blender/makesrna/intern/rna_access.c:2421
    #4 0x55e954c17883 in ui_but_value_get /home/guest/blender/src/source/blender/editors/interface/interface.c:2324
    #5 0x55e954c78d79 in ui_apply_but_TOG /home/guest/blender/src/source/blender/editors/interface/interface_handlers.c:959
    #6 0x55e954c8798e in ui_apply_but /home/guest/blender/src/source/blender/editors/interface/interface_handlers.c:2080
    #7 0x55e954c9fde2 in ui_do_but_ANY_drag_toggle /home/guest/blender/src/source/blender/editors/interface/interface_handlers.c:4148
    #8 0x55e954ca3eb8 in ui_do_but_TOG /home/guest/blender/src/source/blender/editors/interface/interface_handlers.c:4415
    #9 0x55e954cda5cb in ui_do_button /home/guest/blender/src/source/blender/editors/interface/interface_handlers.c:7487
    #10 0x55e954ce71d7 in ui_handle_button_event /home/guest/blender/src/source/blender/editors/interface/interface_handlers.c:8661
    #11 0x55e954cfd5e2 in ui_region_handler /home/guest/blender/src/source/blender/editors/interface/interface_handlers.c:10569
    #12 0x55e9519a475c in wm_handler_ui_call /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:617
    #13 0x55e9519b86b0 in wm_handlers_do_intern /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:2553
    #14 0x55e9519bb354 in wm_handlers_do /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:2799
    #15 0x55e9519c0fd6 in wm_event_do_handlers /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:3252
    #16 0x55e95198f479 in WM_main /home/guest/blender/src/source/blender/windowmanager/intern/wm.c:418
    #17 0x55e950c1a332 in main /home/guest/blender/src/source/creator/creator.c:518
    #18 0x7f2ae0f94bba in __libc_start_main ../csu/libc-start.c:308
    #19 0x55e950c19509 in _start (/home/guest/blender/build_master_debug/bin/blender+0x278b1509)

0x60d0008601b8 is located 24 bytes inside of 136-byte region [0x60d0008601a0,0x60d000860228)
freed by thread T0 here:
    #0 0x7f2ae6269277 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0x107277)
    #1 0x55e961cbfdef in MEM_lockfree_freeN /home/guest/blender/src/intern/guardedalloc/intern/mallocn_lockfree_impl.c:157
    #2 0x55e961aee4d4 in BLI_freelistN /home/guest/blender/src/source/blender/blenlib/intern/listbase.c:554
    #3 0x55e950d67b0c in IDP_FreeGroup /home/guest/blender/src/source/blender/blenkernel/intern/idprop.c:760
    #4 0x55e950d6b918 in IDP_FreePropertyContent_ex /home/guest/blender/src/source/blender/blenkernel/intern/idprop.c:1085
    #5 0x55e950e17df0 in BKE_libblock_free_data /home/guest/blender/src/source/blender/blenkernel/intern/lib_id_delete.c:113
    #6 0x55e950e18cd3 in BKE_id_free_ex /home/guest/blender/src/source/blender/blenkernel/intern/lib_id_delete.c:331
    #7 0x55e950e92d82 in BKE_main_free /home/guest/blender/src/source/blender/blenkernel/intern/main.c:75
    #8 0x55e950c2a817 in BKE_blender_globals_clear /home/guest/blender/src/source/blender/blenkernel/intern/blender.c:148
    #9 0x55e9554c105d in setup_app_data /home/guest/blender/src/source/blender/blenkernel/intern/blendfile.c:259
    #10 0x55e9554c23dc in setup_app_blend_file_data /home/guest/blender/src/source/blender/blenkernel/intern/blendfile.c:394
    #11 0x55e9554c2f88 in BKE_blendfile_read_from_memfile /home/guest/blender/src/source/blender/blenkernel/intern/blendfile.c:491
    #12 0x55e9554bf06c in BKE_memfile_undo_decode /home/guest/blender/src/source/blender/blenkernel/intern/blender_undo.c:87
    #13 0x55e954bf8949 in memfile_undosys_step_decode /home/guest/blender/src/source/blender/editors/undo/memfile_undo.c:188
    #14 0x55e9555898ee in undosys_step_decode /home/guest/blender/src/source/blender/blenkernel/intern/undo_system.c:210
    #15 0x55e95558f34d in BKE_undosys_step_undo_with_data_ex /home/guest/blender/src/source/blender/blenkernel/intern/undo_system.c:684
    #16 0x55e95558f943 in BKE_undosys_step_undo_with_data /home/guest/blender/src/source/blender/blenkernel/intern/undo_system.c:720
    #17 0x55e95558f9ce in BKE_undosys_step_undo /home/guest/blender/src/source/blender/blenkernel/intern/undo_system.c:725
    #18 0x55e954bef8e3 in ed_undo_step_impl /home/guest/blender/src/source/blender/editors/undo/ed_undo.c:190
    #19 0x55e954bf019b in ed_undo_step_direction /home/guest/blender/src/source/blender/editors/undo/ed_undo.c:253
    #20 0x55e954bf144d in ed_undo_exec /home/guest/blender/src/source/blender/editors/undo/ed_undo.c:389
    #21 0x55e9519ab4f1 in wm_operator_invoke /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:1279
    #22 0x55e9519b30b4 in wm_handler_operator_call /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:2091
    #23 0x55e9519b6c67 in wm_handlers_do_keymap_with_keymap_handler /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:2401
    #24 0x55e9519b83e6 in wm_handlers_do_intern /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:2537
    #25 0x55e9519bb354 in wm_handlers_do /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:2799
    #26 0x55e9519c12d6 in wm_event_do_handlers /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:3286
    #27 0x55e95198f479 in WM_main /home/guest/blender/src/source/blender/windowmanager/intern/wm.c:418
    #28 0x55e950c1a332 in main /home/guest/blender/src/source/creator/creator.c:518
    #29 0x7f2ae0f94bba in __libc_start_main ../csu/libc-start.c:308

previously allocated by thread T0 here:
    #0 0x7f2ae6269628 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
    #1 0x55e961cc0859 in MEM_lockfree_mallocN /home/guest/blender/src/intern/guardedalloc/intern/mallocn_lockfree_impl.c:308
    #2 0x55e951ae5d42 in read_struct /home/guest/blender/src/source/blender/blenloader/intern/readfile.c:2294
    #3 0x55e951b5678e in read_data_into_oldnewmap /home/guest/blender/src/source/blender/blenloader/intern/readfile.c:8986
    #4 0x55e951b57886 in read_libblock /home/guest/blender/src/source/blender/blenloader/intern/readfile.c:9103
    #5 0x55e951b61117 in blo_read_file_internal /home/guest/blender/src/source/blender/blenloader/intern/readfile.c:9967
    #6 0x55e951acc29b in BLO_read_from_memfile /home/guest/blender/src/source/blender/blenloader/intern/readblenentry.c:407
    #7 0x55e9554c2ae6 in BKE_blendfile_read_from_memfile /home/guest/blender/src/source/blender/blenkernel/intern/blendfile.c:481
    #8 0x55e9554bf06c in BKE_memfile_undo_decode /home/guest/blender/src/source/blender/blenkernel/intern/blender_undo.c:87
    #9 0x55e954bf8949 in memfile_undosys_step_decode /home/guest/blender/src/source/blender/editors/undo/memfile_undo.c:188
    #10 0x55e9555898ee in undosys_step_decode /home/guest/blender/src/source/blender/blenkernel/intern/undo_system.c:210
    #11 0x55e95558f818 in BKE_undosys_step_undo_with_data_ex /home/guest/blender/src/source/blender/blenkernel/intern/undo_system.c:709
    #12 0x55e95558f943 in BKE_undosys_step_undo_with_data /home/guest/blender/src/source/blender/blenkernel/intern/undo_system.c:720
    #13 0x55e95558f9ce in BKE_undosys_step_undo /home/guest/blender/src/source/blender/blenkernel/intern/undo_system.c:725
    #14 0x55e954bef8e3 in ed_undo_step_impl /home/guest/blender/src/source/blender/editors/undo/ed_undo.c:190
    #15 0x55e954bf019b in ed_undo_step_direction /home/guest/blender/src/source/blender/editors/undo/ed_undo.c:253
    #16 0x55e954bf144d in ed_undo_exec /home/guest/blender/src/source/blender/editors/undo/ed_undo.c:389
    #17 0x55e9519ab4f1 in wm_operator_invoke /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:1279
    #18 0x55e9519b30b4 in wm_handler_operator_call /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:2091
    #19 0x55e9519b6c67 in wm_handlers_do_keymap_with_keymap_handler /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:2401
    #20 0x55e9519b83e6 in wm_handlers_do_intern /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:2537
    #21 0x55e9519bb354 in wm_handlers_do /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:2799
    #22 0x55e9519c12d6 in wm_event_do_handlers /home/guest/blender/src/source/blender/windowmanager/intern/wm_event_system.c:3286
    #23 0x55e95198f479 in WM_main /home/guest/blender/src/source/blender/windowmanager/intern/wm.c:418
    #24 0x55e950c1a332 in main /home/guest/blender/src/source/creator/creator.c:518
    #25 0x7f2ae0f94bba in __libc_start_main ../csu/libc-start.c:308

@Campbell Barton (campbellbarton) think I need your knowledge of undo and/or WM here. Looking in undo code I actually cannot find anything enforcing an immediate full redraw/update of the UI right after undo. And I cannot actually locate any function in WM that would allow this?

We do have an ED_editors_init_for_undo(), but its scope seems very limited...

Note: after undo speedup work is done and in master, this will almost never be an issue anymore, since we'll reuse existing memory... But that will just hide the issue, not actually fix it.

Wayde Moss (GuiltyGhost) added a revision: D7783: Keyframe: Copy To Selected.May 19 2020, 6:34 AM
Wayde Moss (GuiltyGhost) removed a revision: D7783: Keyframe: Copy To Selected.
Julian Eisel (Severin) added a subscriber: Julian Eisel (Severin).May 19 2020, 10:40 AM

We most likely send a fake mouse move event after the button is pressed, to force a redraw and thus updating the hovered button highlight. So I think it's the UI code causing that redraw, not undo code.

Campbell Barton (campbellbarton) claimed this task.EditedMay 19 2020, 1:41 PM

This looks like an issue with the order of event handling, looking into it.

Campbell Barton (campbellbarton) added a comment.EditedMay 19 2020, 2:53 PM

This is a more general issue than undo, this could happen with any operator that frees an ID.

The bug happens rarely because the UI typically refreshes after an operator runs.

This is similar to the problem we ran into where operators needed an evaluated depsgraph, except in this case we need an updated UI.

I don't think this is something we should try solve for 2.83 as it would likely involve changes to event loop handling.

We could change the event system to support early-exit to refresh the UI, then
we could detect operators that free ID's and be sure to refresh before other events are handled.

(Edit, as it's possible for this to happen freeing any non-ID data such as constraints, modifiers etc... we could ensure UI handlers always operate on a interface that's been refreshed after running other non-UI handlers).

Here is a workaround for the bug, showing how exiting the event look early resolves the issue:

P1407 T74067 event handling workaround
1diff --git a/source/blender/editors/undo/ed_undo.c b/source/blender/editors/undo/ed_undo.c
2index 6633e1c427c..4bec8a61884 100644
3--- a/source/blender/editors/undo/ed_undo.c
4+++ b/source/blender/editors/undo/ed_undo.c
5@@ -255,6 +255,9 @@ static int ed_undo_step_impl(
6 BKE_undosys_print(wm->undo_stack);
7 }
8
9+ /* Force break out of the event loop. */
10+ CTX_wm_window_set(C, NULL);
11+
12 return OPERATOR_FINISHED;
13 }
14
15@@ -396,10 +399,11 @@ static int ed_undo_exec(bContext *C, wmOperator *op)
16 {
17 /* "last operator" should disappear, later we can tie this with undo stack nicer */
18 WM_operator_stack_clear(CTX_wm_manager(C));
19+ wmWindow *win = CTX_wm_window(C);
20 int ret = ed_undo_step_direction(C, 1, op->reports);
21 if (ret & OPERATOR_FINISHED) {
22 /* Keep button under the cursor active. */
23- WM_event_add_mousemove(CTX_wm_window(C));
24+ WM_event_add_mousemove(win);
25 }
26
27 ED_outliner_select_sync_from_all_tag(C);
28@@ -425,10 +429,11 @@ static int ed_undo_push_exec(bContext *C, wmOperator *op)
29
30 static int ed_redo_exec(bContext *C, wmOperator *op)
31 {
32+ wmWindow *win = CTX_wm_window(C);
33 int ret = ed_undo_step_direction(C, -1, op->reports);
34 if (ret & OPERATOR_FINISHED) {
35 /* Keep button under the cursor active. */
36- WM_event_add_mousemove(CTX_wm_window(C));
37+ WM_event_add_mousemove(win);
38 }
39
40 ED_outliner_select_sync_from_all_tag(C);
41@@ -437,12 +442,13 @@ static int ed_redo_exec(bContext *C, wmOperator *op)
42
43 static int ed_undo_redo_exec(bContext *C, wmOperator *UNUSED(op))
44 {
45+ wmWindow *win = CTX_wm_window(C);
46 wmOperator *last_op = WM_operator_last_redo(C);
47 int ret = ED_undo_operator_repeat(C, last_op);
48 ret = ret ? OPERATOR_FINISHED : OPERATOR_CANCELLED;
49 if (ret & OPERATOR_FINISHED) {
50 /* Keep button under the cursor active. */
51- WM_event_add_mousemove(CTX_wm_window(C));
52+ WM_event_add_mousemove(win);
53 }
54 return ret;
55 }

This shouldn't be used as-is, because clearing the window is intended for the file-read case, and returning early skips setting the previous event values (prevx/prevy).

Campbell Barton (campbellbarton) added a revision: D7795: Fix T74067: Crash when the UI accesses stale data.May 20 2020, 6:27 AM
Campbell Barton (campbellbarton) mentioned this in D7795: Fix T74067: Crash when the UI accesses stale data.May 20 2020, 6:37 AM
Julian Eisel (Severin) moved this task from Backlog to Bugs on the User Interface board.Jun 26 2020, 5:54 PM
Bastien Montagne (mont29) mentioned this in T80203: Crash when changing torus properties .Oct 16 2020, 11:58 AM
Oliver Weissbarth (oweissbarth) added a subscriber: Oliver Weissbarth (oweissbarth).Apr 5 2021, 5:56 PM